From: Casey Schaufler <casey@schaufler-ca.com> To: Jann Horn <jannh@google.com> Cc: David Howells <dhowells@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>, raven@themaw.net, linux-fsdevel <linux-fsdevel@vger.kernel.org>, Linux API <linux-api@vger.kernel.org>, linux-block@vger.kernel.org, keyrings@vger.kernel.org, linux-security-module <linux-security-module@vger.kernel.org>, kernel list <linux-kernel@vger.kernel.org>, Andy Lutomirski <luto@kernel.org>, casey@schaufler-ca.com Subject: Re: [PATCH 3/7] vfs: Add a mount-notification facility Date: Wed, 29 May 2019 10:04:51 -0700 [thread overview] Message-ID: <c95dd6cd-5530-6b70-68f6-4038edd72352@schaufler-ca.com> (raw) In-Reply-To: <CAG48ez2KMrTBFzO9p8GvduXruz+FNLPyhc2YivHePsgViEoT1g@mail.gmail.com> On 5/29/2019 9:12 AM, Jann Horn wrote: > On Wed, May 29, 2019 at 5:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 5/29/2019 4:00 AM, David Howells wrote: >>> Jann Horn <jannh@google.com> wrote: >>> >>>>> +void post_mount_notification(struct mount *changed, >>>>> + struct mount_notification *notify) >>>>> +{ >>>>> + const struct cred *cred = current_cred(); >>>> This current_cred() looks bogus to me. Can't mount topology changes >>>> come from all sorts of places? For example, umount_mnt() from >>>> umount_tree() from dissolve_on_fput() from __fput(), which could >>>> happen pretty much anywhere depending on where the last reference gets >>>> dropped? >>> IIRC, that's what Casey argued is the right thing to do from a security PoV. >>> Casey? >> You need to identify the credential of the subject that triggered >> the event. If it isn't current_cred(), the cred needs to be passed >> in to post_mount_notification(), or derived by some other means. >> >>> Maybe I should pass in NULL creds in the case that an event is being generated >>> because an object is being destroyed due to the last usage[*] being removed. >> You should pass the cred of the process that removed the >> last usage. If the last usage was removed by something like >> the power being turned off on a disk drive a system cred >> should be used. Someone or something caused the event. It can >> be important who it was. > The kernel's normal security model means that you should be able to > e.g. accept FDs that random processes send you and perform > read()/write() calls on them without acting as a subject in any > security checks; let alone close(). Passed file descriptors are an anomaly in the security model that (in this developer's opinion) should have never been included. More than one of the "B" level UNIX systems disabled them outright. > If you send a file descriptor over > a unix domain socket and the unix domain socket is garbage collected, > for example, I think the close() will just come from some random, > completely unrelated task that happens to trigger the garbage > collector? I never said this was going to be easy or pleasant. Who destroyed the UDS? It didn't just spontaneously become garbage. Well, not on modern Linux filesystems, anyway. > Also, I think if someone does I/O via io_uring, I think the caller's > credentials for read/write operations will probably just be normal > kernel creds? > > Here the checks probably aren't all that important, but in other > places, when people try to use an LSM as the primary line of defense, > checks that don't align with the kernel's normal security model might > lead to a bunch of problems. The kernel does not have a "normal security model". It has a collection of disparate and almost but not quite contradictory models for the various objects and mechanisms it implements. It already has a bunch of problems, we're just used to them. I can only send a signal to a process with the same UID. Why doesn't a process have mode bits so that I could get signals from my group? Why do IPC object have creator bits, while files don't? Why can I send a file descriptor over a UDS, but not a message queue? Why can't I set the mode bits on a symlink? What can go wrong if I don't map groups into a user namespace? LSMs (SELinux and Smack, which are classic mandatory access control systems in particular) are more consistent, but still have to deal with some of these differences. A symlink gets a Smack label, for example. The point being that it's very easy to add new mechanisms that do wonderful things but that introduce unforeseen ways to bypass one or more of the existing protections.
WARNING: multiple messages have this Message-ID (diff)
From: Casey Schaufler <casey@schaufler-ca.com> To: Jann Horn <jannh@google.com> Cc: David Howells <dhowells@redhat.com>, Al Viro <viro@zeniv.linux.org.uk>, raven@themaw.net, linux-fsdevel <linux-fsdevel@vger.kernel.org>, Linux API <linux-api@vger.kernel.org>, linux-block@vger.kernel.org, keyrings@vger.kernel.org, linux-security-module <linux-security-module@vger.kernel.org>, kernel list <linux-kernel@vger.kernel.org>, Andy Lutomirski <luto@kernel.org>, casey@schaufler-ca.com Subject: Re: [PATCH 3/7] vfs: Add a mount-notification facility Date: Wed, 29 May 2019 17:04:51 +0000 [thread overview] Message-ID: <c95dd6cd-5530-6b70-68f6-4038edd72352@schaufler-ca.com> (raw) In-Reply-To: <CAG48ez2KMrTBFzO9p8GvduXruz+FNLPyhc2YivHePsgViEoT1g@mail.gmail.com> On 5/29/2019 9:12 AM, Jann Horn wrote: > On Wed, May 29, 2019 at 5:53 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >> On 5/29/2019 4:00 AM, David Howells wrote: >>> Jann Horn <jannh@google.com> wrote: >>> >>>>> +void post_mount_notification(struct mount *changed, >>>>> + struct mount_notification *notify) >>>>> +{ >>>>> + const struct cred *cred = current_cred(); >>>> This current_cred() looks bogus to me. Can't mount topology changes >>>> come from all sorts of places? For example, umount_mnt() from >>>> umount_tree() from dissolve_on_fput() from __fput(), which could >>>> happen pretty much anywhere depending on where the last reference gets >>>> dropped? >>> IIRC, that's what Casey argued is the right thing to do from a security PoV. >>> Casey? >> You need to identify the credential of the subject that triggered >> the event. If it isn't current_cred(), the cred needs to be passed >> in to post_mount_notification(), or derived by some other means. >> >>> Maybe I should pass in NULL creds in the case that an event is being generated >>> because an object is being destroyed due to the last usage[*] being removed. >> You should pass the cred of the process that removed the >> last usage. If the last usage was removed by something like >> the power being turned off on a disk drive a system cred >> should be used. Someone or something caused the event. It can >> be important who it was. > The kernel's normal security model means that you should be able to > e.g. accept FDs that random processes send you and perform > read()/write() calls on them without acting as a subject in any > security checks; let alone close(). Passed file descriptors are an anomaly in the security model that (in this developer's opinion) should have never been included. More than one of the "B" level UNIX systems disabled them outright. > If you send a file descriptor over > a unix domain socket and the unix domain socket is garbage collected, > for example, I think the close() will just come from some random, > completely unrelated task that happens to trigger the garbage > collector? I never said this was going to be easy or pleasant. Who destroyed the UDS? It didn't just spontaneously become garbage. Well, not on modern Linux filesystems, anyway. > Also, I think if someone does I/O via io_uring, I think the caller's > credentials for read/write operations will probably just be normal > kernel creds? > > Here the checks probably aren't all that important, but in other > places, when people try to use an LSM as the primary line of defense, > checks that don't align with the kernel's normal security model might > lead to a bunch of problems. The kernel does not have a "normal security model". It has a collection of disparate and almost but not quite contradictory models for the various objects and mechanisms it implements. It already has a bunch of problems, we're just used to them. I can only send a signal to a process with the same UID. Why doesn't a process have mode bits so that I could get signals from my group? Why do IPC object have creator bits, while files don't? Why can I send a file descriptor over a UDS, but not a message queue? Why can't I set the mode bits on a symlink? What can go wrong if I don't map groups into a user namespace? LSMs (SELinux and Smack, which are classic mandatory access control systems in particular) are more consistent, but still have to deal with some of these differences. A symlink gets a Smack label, for example. The point being that it's very easy to add new mechanisms that do wonderful things but that introduce unforeseen ways to bypass one or more of the existing protections.
next prev parent reply other threads:[~2019-05-29 17:04 UTC|newest] Thread overview: 131+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-05-28 16:01 [RFC][PATCH 0/7] Mount, FS, Block and Keyrings notifications David Howells 2019-05-28 16:01 ` David Howells 2019-05-28 16:01 ` [PATCH 1/7] General notification queue with user mmap()'able ring buffer David Howells 2019-05-28 16:01 ` David Howells 2019-05-28 16:26 ` Greg KH 2019-05-28 16:26 ` Greg KH 2019-05-28 17:30 ` David Howells 2019-05-28 17:30 ` David Howells 2019-05-28 23:12 ` Greg KH 2019-05-28 23:12 ` Greg KH 2019-05-29 16:06 ` David Howells 2019-05-29 16:06 ` David Howells 2019-05-29 17:46 ` Jann Horn 2019-05-29 17:46 ` Jann Horn 2019-05-29 21:02 ` David Howells 2019-05-29 21:02 ` David Howells 2019-05-31 11:14 ` Peter Zijlstra 2019-05-31 11:14 ` Peter Zijlstra 2019-05-31 12:02 ` David Howells 2019-05-31 12:02 ` David Howells 2019-05-31 13:26 ` Peter Zijlstra 2019-05-31 13:26 ` Peter Zijlstra 2019-05-31 14:20 ` David Howells 2019-05-31 14:20 ` David Howells 2019-05-31 16:44 ` Peter Zijlstra 2019-05-31 16:44 ` Peter Zijlstra 2019-05-31 17:12 ` David Howells 2019-05-31 17:12 ` David Howells 2019-06-17 16:24 ` Peter Zijlstra 2019-06-17 16:24 ` Peter Zijlstra 2019-05-29 23:09 ` Greg KH 2019-05-29 23:09 ` Greg KH 2019-05-29 23:11 ` Greg KH 2019-05-29 23:11 ` Greg KH 2019-05-30 9:50 ` Andrea Parri 2019-05-30 9:50 ` Andrea Parri 2019-05-31 8:35 ` Peter Zijlstra 2019-05-31 8:35 ` Peter Zijlstra 2019-05-31 8:47 ` Peter Zijlstra 2019-05-31 8:47 ` Peter Zijlstra 2019-05-31 12:42 ` David Howells 2019-05-31 12:42 ` David Howells 2019-05-31 14:55 ` David Howells 2019-05-31 14:55 ` David Howells 2019-05-28 19:14 ` Jann Horn 2019-05-28 19:14 ` Jann Horn 2019-05-28 22:28 ` David Howells 2019-05-28 22:28 ` David Howells 2019-05-28 23:16 ` Jann Horn 2019-05-28 23:16 ` Jann Horn 2019-05-28 16:02 ` [PATCH 2/7] keys: Add a notification facility David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 16:02 ` [PATCH 3/7] vfs: Add a mount-notification facility David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 20:06 ` Jann Horn 2019-05-28 20:06 ` Jann Horn 2019-05-28 23:04 ` David Howells 2019-05-28 23:04 ` David Howells 2019-05-28 23:23 ` Jann Horn 2019-05-28 23:23 ` Jann Horn 2019-05-29 11:16 ` David Howells 2019-05-29 11:16 ` David Howells 2019-05-28 23:08 ` David Howells 2019-05-28 23:08 ` David Howells 2019-05-29 10:55 ` David Howells 2019-05-29 10:55 ` David Howells 2019-05-29 11:00 ` David Howells 2019-05-29 11:00 ` David Howells 2019-05-29 15:53 ` Casey Schaufler 2019-05-29 15:53 ` Casey Schaufler 2019-05-29 16:12 ` Jann Horn 2019-05-29 16:12 ` Jann Horn 2019-05-29 17:04 ` Casey Schaufler [this message] 2019-05-29 17:04 ` Casey Schaufler 2019-06-03 16:30 ` David Howells 2019-06-03 16:30 ` David Howells 2019-05-29 17:13 ` Andy Lutomirski 2019-05-29 17:13 ` Andy Lutomirski 2019-05-29 17:46 ` Casey Schaufler 2019-05-29 17:46 ` Casey Schaufler 2019-05-29 18:11 ` Jann Horn 2019-05-29 18:11 ` Jann Horn 2019-05-29 19:28 ` Casey Schaufler 2019-05-29 19:28 ` Casey Schaufler 2019-05-29 19:47 ` Jann Horn 2019-05-29 19:47 ` Jann Horn 2019-05-29 20:50 ` Casey Schaufler 2019-05-29 20:50 ` Casey Schaufler 2019-05-29 23:12 ` Andy Lutomirski 2019-05-29 23:12 ` Andy Lutomirski 2019-05-29 23:56 ` Casey Schaufler 2019-05-29 23:56 ` Casey Schaufler 2019-05-28 16:02 ` [PATCH 4/7] vfs: Add superblock notifications David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 20:27 ` Jann Horn 2019-05-28 20:27 ` Jann Horn 2019-05-29 12:58 ` David Howells 2019-05-29 12:58 ` David Howells 2019-05-29 14:16 ` Jann Horn 2019-05-29 14:16 ` Jann Horn 2019-05-28 16:02 ` [PATCH 5/7] fsinfo: Export superblock notification counter David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 16:02 ` [PATCH 6/7] block: Add block layer notifications David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 20:37 ` Jann Horn 2019-05-28 20:37 ` Jann Horn 2019-05-28 16:02 ` [PATCH 7/7] Add sample notification program David Howells 2019-05-28 16:02 ` David Howells 2019-05-28 23:58 ` [RFC][PATCH 0/7] Mount, FS, Block and Keyrings notifications Greg KH 2019-05-28 23:58 ` Greg KH 2019-05-29 6:33 ` Amir Goldstein 2019-05-29 6:33 ` Amir Goldstein 2019-05-29 6:33 ` Amir Goldstein 2019-05-29 14:25 ` Jan Kara 2019-05-29 14:25 ` Jan Kara 2019-05-29 15:10 ` Greg KH 2019-05-29 15:10 ` Greg KH 2019-05-29 15:53 ` Amir Goldstein 2019-05-29 15:53 ` Amir Goldstein 2019-05-30 11:00 ` Jan Kara 2019-05-30 11:00 ` Jan Kara 2019-06-04 12:33 ` David Howells 2019-06-04 12:33 ` David Howells 2019-05-29 6:45 ` David Howells 2019-05-29 6:45 ` David Howells 2019-05-29 7:40 ` Amir Goldstein 2019-05-29 7:40 ` Amir Goldstein 2019-05-29 9:09 ` David Howells 2019-05-29 9:09 ` David Howells 2019-05-29 15:41 ` Casey Schaufler 2019-05-29 15:41 ` Casey Schaufler
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=c95dd6cd-5530-6b70-68f6-4038edd72352@schaufler-ca.com \ --to=casey@schaufler-ca.com \ --cc=dhowells@redhat.com \ --cc=jannh@google.com \ --cc=keyrings@vger.kernel.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=luto@kernel.org \ --cc=raven@themaw.net \ --cc=viro@zeniv.linux.org.uk \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.