linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Stephen Smalley <sds@tycho.nsa.gov>
Cc: dhowells@redhat.com, viro@zeniv.linux.org.uk,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Casey Schaufler <casey@schaufler-ca.com>,
	linux-usb@vger.kernel.org, raven@themaw.net,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	linux-block@vger.kernel.org, keyrings@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org, Paul Moore <paul@paul-moore.com>
Subject: Re: [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3]
Date: Thu, 06 Jun 2019 14:16:43 +0100	[thread overview]
Message-ID: <3813.1559827003@warthog.procyon.org.uk> (raw)
In-Reply-To: <b91710d8-cd2d-6b93-8619-130b9d15983d@tycho.nsa.gov>

Stephen Smalley <sds@tycho.nsa.gov> wrote:

This might be easier to discuss if you can reply to:

	https://lore.kernel.org/lkml/5393.1559768763@warthog.procyon.org.uk/

which is on the ver #2 posting of this patchset.

> > LSM support is included, but controversial:
> >
> >   (1) The creds of the process that did the fput() that reduced the refcount
> >       to zero are cached in the file struct.
> >
> >   (2) __fput() overrides the current creds with the creds from (1) whilst
> >       doing the cleanup, thereby making sure that the creds seen by the
> >       destruction notification generated by mntput() appears to come from
> >       the last fputter.
> >
> >   (3) security_post_notification() is called for each queue that we might
> >       want to post a notification into, thereby allowing the LSM to prevent
> >       covert communications.
> >
> >   (?) Do I need to add security_set_watch(), say, to rule on whether a watch
> >       may be set in the first place?  I might need to add a variant per
> >       watch-type.
> >
> >   (?) Do I really need to keep track of the process creds in which an
> >       implicit object destruction happened?  For example, imagine you create
> >       an fd with fsopen()/fsmount().  It is marked to dissolve the mount it
> >       refers to on close unless move_mount() clears that flag.  Now, imagine
> >       someone looking at that fd through procfs at the same time as you exit
> >       due to an error.  The LSM sees the destruction notification come from
> >       the looker if they happen to do their fput() after yours.
> 
> 
> I'm not in favor of this approach.

Which bit?  The last point?  Keeping track of the process creds after an
implicit object destruction.

> Can we check permission to the object being watched when a watch is set
> (read-like access),

Yes, and I need to do that.  I think it's likely to require an extra hook for
each entry point added because the objects are different:

	int security_watch_key(struct watch *watch, struct key *key);
	int security_watch_sb(struct watch *watch, struct path *path);
	int security_watch_mount(struct watch *watch, struct path *path);
	int security_watch_devices(struct watch *watch);

> make sure every access that can trigger a notification requires a
> (write-like) permission to the accessed object,

"write-like permssion" for whom?  The triggerer or the watcher?

There are various 'classes' of events:

 (1) System events (eg. hardware I/O errors, automount points expiring).

 (2) Direct events (eg. automounts, manual mounts, EDQUOT, key linkage).

 (3) Indirect events (eg. exit/close doing the last fput and causing an
     unmount).

Class (1) are uncaused by a process, so I use init_cred for them.  One could
argue that the automount point expiry should perhaps take place under the
creds of whoever triggered it in the first place, but we need to be careful
about long-term cred pinning.

Class (2) the causing process must've had permission to cause them - otherwise
we wouldn't have got the event.

Class (3) is interesting since it's currently entirely cleanup events and the
process may have the right to do them (close, dup2, exit, but also execve)
whether the LSM thinks it should be able to cause the object to be destroyed
or not.

It gets more complicated than that, though: multiple processes with different
security attributes can all have fds pointing to a common file object - and
the last one to close carries the can as far as the LSM is concerned.

And yet more complicated when you throw in unix sockets with partially passed
fds still in their queues.  That's what patch 01 is designed to try and cope
with.

> and make sure there is some sane way to control the relationship between the
> accessed object and the watched object (write-like)?

This is the trick.  Keys and superblocks have object labels of their own and
don't - for now - propagate their watches.  With these, the watch is on the
object you initially assign it to and it goes no further than that.

mount_notify() is the interesting case since we want to be able to detect
mount topology change events from within the vfs subtree rooted at the watched
directory without having to manually put a watch on every directory in that
subtree - or even just every mount object.

Or, maybe, that's what I'll have to do: make it mount_notify() can only apply
to the subtree within its superblock, and the caller must call mount_notify()
for every mount object it wants to monitor.  That would at least ensure that
the caller can, at that point, reach all those mount points.

> For cases where we have no object per se or at least no security
> structure/label associated with it, we may have to fall back to a
> coarse-grained "Can the watcher get this kind of notification in general?".

Agreed - and we should probably have that anyway.

David

  parent reply	other threads:[~2019-06-06 13:17 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-06  9:41 [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications [ver #3] David Howells
2019-06-06  9:42 ` [PATCH 01/10] security: Override creds in __fput() with last fputter's creds " David Howells
2019-06-06 14:57   ` Andy Lutomirski
2019-06-06 15:06   ` David Howells
2019-06-06 17:18     ` Andy Lutomirski
2019-06-06 19:09       ` Casey Schaufler
2019-06-06 19:34         ` Andy Lutomirski
2019-06-06  9:42 ` [PATCH 02/10] General notification queue with user mmap()'able ring buffer " David Howells
2019-06-06  9:42 ` [PATCH 03/10] keys: Add a notification facility " David Howells
2019-06-06  9:42 ` [PATCH 04/10] vfs: Add a mount-notification " David Howells
2019-06-06  9:42 ` [PATCH 05/10] vfs: Add superblock notifications " David Howells
2019-06-06  9:42 ` [PATCH 06/10] fsinfo: Export superblock notification counter " David Howells
2019-06-06  9:43 ` [PATCH 07/10] Add a general, global device notification watch list " David Howells
2019-06-06  9:43 ` [PATCH 08/10] block: Add block layer notifications " David Howells
2019-06-06  9:43 ` [PATCH 09/10] usb: Add USB subsystem " David Howells
2019-06-06 14:24   ` Alan Stern
2019-06-06 14:33     ` Greg Kroah-Hartman
2019-06-06 14:55       ` Alan Stern
2019-06-06 15:31         ` Greg Kroah-Hartman
2019-06-07  6:40           ` Felipe Balbi
2019-06-07 14:01             ` Alan Stern
2019-06-11  6:28               ` Felipe Balbi
2019-06-11 13:53                 ` Alan Stern
2019-06-12  6:58                   ` Felipe Balbi
2019-06-06  9:43 ` [PATCH 10/10] Add sample notification program " David Howells
2019-06-06 21:21   ` Eugeniu Rosca
2019-06-06 22:52   ` David Howells
2019-06-07 14:37   ` David Howells
2019-06-06 12:32 ` [RFC][PATCH 00/10] Mount, FS, Block and Keyrings notifications " Stephen Smalley
2019-06-06 13:16 ` David Howells [this message]
2019-06-06 14:05   ` Stephen Smalley
2019-06-06 16:43     ` Casey Schaufler
2019-06-06 17:11       ` Andy Lutomirski
2019-06-06 18:33         ` Casey Schaufler
2019-06-06 18:51           ` Andy Lutomirski
2019-06-06 17:16       ` Stephen Smalley
2019-06-06 18:56         ` Casey Schaufler
2019-06-06 19:54           ` Andy Lutomirski
2019-06-06 21:17           ` David Howells
2019-06-06 21:54             ` Andy Lutomirski
2019-06-06 22:38             ` David Howells
2019-06-06 22:42               ` Andy Lutomirski
2019-06-06 22:50               ` David Howells
2019-06-06 14:34 ` Christian Brauner

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=3813.1559827003@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=casey@schaufler-ca.com \
    --cc=gregkh@linuxfoundation.org \
    --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=linux-usb@vger.kernel.org \
    --cc=paul@paul-moore.com \
    --cc=raven@themaw.net \
    --cc=sds@tycho.nsa.gov \
    --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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).