On Thu, Jun 6, 2019 at 9:43 AM Casey Schaufler [off-list ref] wrote:

On 6/6/2019 7:05 AM, Stephen Smalley wrote:

quoted

On 6/6/19 9:16 AM, David Howells wrote:

quoted

Stephen Smalley [off-list ref] wrote:

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

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

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

Sorry for being late to the party.  Not sure whether you're asking me to respond only there or both there and here to your comments below.  I'll start here but can revisit if it's a problem.

quoted

quoted

quoted

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.

(1), (2), (3), and the last point.

quoted

quoted

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);

quoted

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?

The former, i.e. the process that performed the operation that triggered the notification.  Think of it as a write from the process to the accessed object, which triggers a notification (another write) on some related object (the watched object), which is then read by the watcher.

We agree that the process that performed the operation that triggered
the notification is the initial subject. Smack will treat the process
with the watch set (in particular, its ring buffer) as the object
being written to. SELinux, with its finer grained controls, will
involve other things as noted above. There are other place where we
see this, notably IP packet delivery.

The implication is that the information about the triggering
process needs to be available throughout.

quoted

quoted

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.

This seems equivalent to just checking whether the watcher is allowed to get that kind of event, no other cred truly needed.

quoted

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

So we've already done a check on the causing process, and we're going to check whether the watcher can set the watch. We just need to establish the connection between the accessed object and the watched object in some manner.

I don't agree. That is, I don't believe it is sufficient.
There is no guarantee that being able to set a watch on an
object implies that every process that can trigger the event
can send it to you.

        Watcher has Smack label W
        Triggerer has Smack label T
        Watched object has Smack label O

        Relevant Smack rules are

        W O rw
        T O rw

The watcher will be able to set the watch,
the triggerer will be able to trigger the event,
but there is nothing that would allow the watcher
to receive the event. This is not a case of watcher
reading the watched object, as the event is delivered
without any action by watcher.

I think this is an example of a bogus policy that should not be
supported by the kernel.