On 3/8/2024 12:12 PM, Mickaël Salaün wrote: > On Fri, Mar 08, 2024 at 02:22:58PM -0500, Paul Moore wrote: >> On Fri, Mar 8, 2024 at 4:29 AM Mickaël Salaün <mic@xxxxxxxxxxx> wrote: >>> 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". >> Which is a problem from a LSM perspective as we want to avoid hooks >> which are tightly bound to a single LSM or security model. It's okay >> if a new hook only has a single LSM implementation, but the hook's >> definition should be such that it is reasonably generalized to support >> multiple LSM/models. I've been watching this thread with some interest, as one of my side projects has been an attempt to address the "CAP_SYS_ADMIN problem", and there looks to be a lot of similarity between that and the "ioctl problem". In both cases it comes down to a matter of: 1. uniquely identifying the action 2. providing the information to code that can act upon it 3. providing "policy" to determine what to do about it My thought for the CAP_SYS_ADMIN case was to provide a new LSM hook security_sysadmin() that takes a single parameter which is the action ID. I called the action ID a "chit", because it's short and all the good, more descriptive words where taken. Calls to cap_able(CAP_SYS_ADMIN) could be replaced by calls to security_sysadmin(chit). security_sysadmin() would first call cap_able(CAP_SYSADMIN) and, if that succeeded, allow LSMs with registered hooks (selinux_sysadmin() etc) the opportunity to disallow the operation. I planned to include a small LSM (chits) that would allow the operation only if the process had the chit on its chit list. Landlock could add policy to deal with chits if so inclined. A generalization of this scheme would be to leave the cap_able(CAP_SYS_ADMIN) checks as they are and add an optional security_chit() hook for places where additional enforcement information is desired. Adding security_chit(CHIT_IOCTL_TTY_SOMETHING) to an ioctl would allow any LSM to make policy decisions about that ioctl operation. Adding security_chit(CHIT_ERASE_TAPE_REGISTERS) after a cap_able(CAP_SYS_ADMIN) could appease the driver writer who would otherwise be begging for CAP_ERASE_TAPE_REGISTERS. My biggest concern with this scheme is the management of chit values, which would have to be kept in a uapi header. A major advantage of this is that the security_chit() calls would only have to be added where someone wants to take advantage of the mechanism. People who are happy with CAP_SYS_ADMIN or ioctl as it is don't have to do anything, and their code won't get churned for the new world order. The downside is the potentially onerous process of deciding if an LSM cares about an action known only by its number. I have patches close to ready. The chit LSM isn't passing the laugh test quite yet, so I'm holding it back for now. I wanted to bring this up before we go too far down a more complicated path.
