So that the LSM can see the credentials of the last process to do an fput()
on a file object when the file object is being dismantled, do the following
steps:
(1) Cache the current credentials in file->f_fput_cred at the point the
file object's reference count reaches zero.
(2) In __fput(), use override_creds() to apply those credentials to the
dismantling process. This is necessary so that if we're dismantling a
unix socket that has semi-passed fds still in it, their fputs will
pick up the same credentials if they're reduced to zero at that point.
Note that it's probably not strictly necessary to take an extra ref on
the creds here (which override_creds() does).
(3) Destroy the fput creds in file_free_rcu().
This additionally makes the creds available to:
fsnotify
eventpoll
file locking
->fasync, ->release file ops
superblock destruction
mountpoint destruction
This allows various notifications about object cleanups/destructions to
carry appropriate credentials for the LSM to approve/disapprove them based
on the process that caused them, even if indirectly.
Note that this means that someone looking at /proc/<pid>/fd/<n> may end up
being inadvertently noted as the subject of a cleanup message if the
process they're looking at croaks whilst they're looking at it.
Further, kernel services like nfsd and cachefiles may be seen as the
fputter and may not have a system credential. In cachefiles's case, it may
appear that cachefilesd caused the notification.
Suggested-by: Casey Schaufler <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Casey Schaufler <[email protected]>
---
fs/file_table.c | 12 ++++++++++++
include/linux/fs.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/fs/file_table.c b/fs/file_table.c
index 3f9c1b452c1d..9bf2be45b7f9 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -46,6 +46,7 @@ static void file_free_rcu(struct rcu_head *head)
struct file *f = container_of(head, struct file, f_u.fu_rcuhead);
put_cred(f->f_cred);
+ put_cred(f->f_fput_cred);
kmem_cache_free(filp_cachep, f);
}
@@ -252,6 +253,7 @@ struct file *alloc_file_clone(struct file *base, int flags,
*/
static void __fput(struct file *file)
{
+ const struct cred *saved_cred;
struct dentry *dentry = file->f_path.dentry;
struct vfsmount *mnt = file->f_path.mnt;
struct inode *inode = file->f_inode;
@@ -262,6 +264,12 @@ static void __fput(struct file *file)
might_sleep();
+ /* Set the creds of whoever triggered the last fput for the LSM. Note
+ * that this has to be made available to further fputs, say on fds
+ * trapped in a unix socket.
+ */
+ saved_cred = override_creds(file->f_fput_cred);
+
fsnotify_close(file);
/*
* The function eventpoll_release() should be the first called
@@ -293,6 +301,8 @@ static void __fput(struct file *file)
if (unlikely(mode & FMODE_NEED_UNMOUNT))
dissolve_on_fput(mnt);
mntput(mnt);
+
+ revert_creds(saved_cred);
out:
file_free(file);
}
@@ -334,6 +344,7 @@ void fput_many(struct file *file, unsigned int refs)
if (atomic_long_sub_and_test(refs, &file->f_count)) {
struct task_struct *task = current;
+ file->f_fput_cred = get_current_cred();
if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) {
init_task_work(&file->f_u.fu_rcuhead, ____fput);
if (!task_work_add(task, &file->f_u.fu_rcuhead, true))
@@ -368,6 +379,7 @@ void __fput_sync(struct file *file)
if (atomic_long_dec_and_test(&file->f_count)) {
struct task_struct *task = current;
BUG_ON(!(task->flags & PF_KTHREAD));
+ file->f_fput_cred = get_current_cred();
__fput(file);
}
}
diff --git a/include/linux/fs.h b/include/linux/fs.h
index f1c74596cd77..db05738b1951 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -943,6 +943,7 @@ struct file {
loff_t f_pos;
struct fown_struct f_owner;
const struct cred *f_cred;
+ const struct cred *f_fput_cred; /* Who did the last fput() (for LSM) */
struct file_ra_state f_ra;
u64 f_version;
> On Jun 6, 2019, at 2:42 AM, David Howells <[email protected]> wrote:
>
> So that the LSM can see the credentials of the last process to do an fput()
> on a file object when the file object is being dismantled, do the following
> steps:
>
I still maintain that this is a giant design error. Can someone at least come up with a single valid use case that isn’t entirely full of bugs?
Andy Lutomirski <[email protected]> wrote:
> > So that the LSM can see the credentials of the last process to do an fput()
> > on a file object when the file object is being dismantled, do the following
> > steps:
> >
>
> I still maintain that this is a giant design error.
Yes, I know. This was primarily a post so that Greg could play with the USB
notifications stuff I added. The LSM support isn't resolved and is unchanged.
> Can someone at least come up with a single valid use case that isn't
> entirely full of bugs?
"Entirely full of bugs"?
How would you propose I deal with Casey's requirement? I'm getting the
feeling you're going to nak it if I try to fulfil that and he's going to nak
it if I don't.
David
On Thu, Jun 6, 2019 at 8:06 AM David Howells <[email protected]> wrote:
>
> Andy Lutomirski <[email protected]> wrote:
>
> > > So that the LSM can see the credentials of the last process to do an fput()
> > > on a file object when the file object is being dismantled, do the following
> > > steps:
> > >
> >
> > I still maintain that this is a giant design error.
>
> Yes, I know. This was primarily a post so that Greg could play with the USB
> notifications stuff I added. The LSM support isn't resolved and is unchanged.
>
> > Can someone at least come up with a single valid use case that isn't
> > entirely full of bugs?
>
> "Entirely full of bugs"?
I can say "hey, I have this policy that the person who triggered an
event needs such-and-such permission, otherwise the event gets
suppressed". But this isn't a full use case, and it's buggy. It's
not a full use case because I haven't specified what my actual goal is
and why this particular policy achieves my goals. And it's entirely
full of bugs because, as this patch so nicely illustrates, it's not
well defined who triggered the event. For example, if I exec a setuid
process, who triggers the close? What if I send the fd to systemd
over a socket and immediately close my copy before systemd gets (and
ignores) the message? Or if I send it to Wayland, or to any other
process?
A file is closed when everyone is done with it. Trying to figure out
who the last intentional user of the file was seems little better than
random guessing. Defining a security policy based on it seems like a
poor idea.
>
> How would you propose I deal with Casey's requirement? I'm getting the
> feeling you're going to nak it if I try to fulfil that and he's going to nak
> it if I don't.
>
Casey, I think you need to state your requirement in a way that's well
defined, and I think you need to make a compelling case that your
requirement is indeed worth dictating the design of parts of the
kernel outside LSM.
On 6/6/2019 10:18 AM, Andy Lutomirski wrote:
> On Thu, Jun 6, 2019 at 8:06 AM David Howells <[email protected]> wrote:
>> Andy Lutomirski <[email protected]> wrote:
>>
>>>> So that the LSM can see the credentials of the last process to do an fput()
>>>> on a file object when the file object is being dismantled, do the following
>>>> steps:
>>>>
>>> I still maintain that this is a giant design error.
>> Yes, I know. This was primarily a post so that Greg could play with the USB
>> notifications stuff I added. The LSM support isn't resolved and is unchanged.
>>
>>> Can someone at least come up with a single valid use case that isn't
>>> entirely full of bugs?
>> "Entirely full of bugs"?
> I can say "hey, I have this policy that the person who triggered an
> event needs such-and-such permission, otherwise the event gets
> suppressed". But this isn't a full use case, and it's buggy. It's
> not a full use case because I haven't specified what my actual goal is
> and why this particular policy achieves my goals. And it's entirely
> full of bugs because, as this patch so nicely illustrates, it's not
> well defined who triggered the event. For example, if I exec a setuid
> process, who triggers the close? What if I send the fd to systemd
> over a socket and immediately close my copy before systemd gets (and
> ignores) the message? Or if I send it to Wayland, or to any other
> process?
>
> A file is closed when everyone is done with it. Trying to figure out
> who the last intentional user of the file was seems little better than
> random guessing. Defining a security policy based on it seems like a
> poor idea.
>
>> How would you propose I deal with Casey's requirement? I'm getting the
>> feeling you're going to nak it if I try to fulfil that and he's going to nak
>> it if I don't.
>>
> Casey, I think you need to state your requirement in a way that's well
> defined, and I think you need to make a compelling case that your
> requirement is indeed worth dictating the design of parts of the
> kernel outside LSM.
Err, no, I don't believe so. There's a whole lot more
going on in this discussion than just what's going on
within the LSMs. Using examples from the LSMs makes it
easier, because their policies are better defined than
the "legacy" policies are. The most important part of the
discussion is about ensuring that the event mechanism
doesn't circumvent the legacy policies. Yes, I understand
that you don't know what that means, or has to do with
anything.
On Thu, Jun 6, 2019 at 12:09 PM Casey Schaufler <[email protected]> wrote:
>
> On 6/6/2019 10:18 AM, Andy Lutomirski wrote:
> > On Thu, Jun 6, 2019 at 8:06 AM David Howells <[email protected]> wrote:
> >> Andy Lutomirski <[email protected]> wrote:
> > Casey, I think you need to state your requirement in a way that's well
> > defined, and I think you need to make a compelling case that your
> > requirement is indeed worth dictating the design of parts of the
> > kernel outside LSM.
>
> Err, no, I don't believe so. There's a whole lot more
> going on in this discussion than just what's going on
> within the LSMs. Using examples from the LSMs makes it
> easier, because their policies are better defined than
> the "legacy" policies are. The most important part of the
> discussion is about ensuring that the event mechanism
> doesn't circumvent the legacy policies. Yes, I understand
> that you don't know what that means, or has to do with
> anything.
>
>
Indeed, I do not know what you have in mind about making sure this
mechanism doesn't circumvent legacy policies. Can you elaborate?
--Andy