Re: What do LSMs *actually* need for checks on notifications?

[Casey Schaufler]

On 6/12/2019 10:41 AM, David Howells wrote:
> Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>
>>>  (4) The security attributes of the object on which the watch was set (uid,
>>>      gid, mode, labels).
>> Smack needs this to set a watch on the named object (file, key, ...).
>> I am going to say that if you can't access an object you can't watch it.
> So for the things I've so far defined:
>
>  (*) Keys/keyrings require View permission, but it could be Read permission
>      instead - though that may get disabled if the key type does not support
>      KEYCTL_READ.

View is good enough.

>  (*) Mount/superblock watches - I've made these require execute permission on
>      the specified directory.  Could be read permission instead.

Execute is good enough.

>  (*) Device events (block/usb) don't require any permissions, but currently
>      only deliver hardware notifications.

How do you specify what device you want to watch?
Don't you have to access a /dev/something?

"currently" makes me nervous.

>> I think that read access is sufficient provided that no one else can
>> see what watches I've created.
> You can't find out what watches exist.

Not even your own?


>>> At the moment, when post_one_notification() wants to write a notification
>>> into a queue, it calls security_post_notification() to ask if it should be
>>> allowed to do so.  This is passed (1) and (3) above plus the notification
>>> record.
>> Is "current" (2)? Smack needs (2) for the event delivery access check.
> (2) was current_cred() when watch_sb(), KEYCTL_NOTIFY, etc. was called, but
> isn't necessarily current_cred() at the point post_one_notification() is
> called.  The latter is called at the point the event is generated and
> current_cred() is the creds of whatever thread that is called in (which may be
> a work_queue thread if it got deferred).
>
> At the moment, I'm storing the creds of whoever opened the queue (ie. (1)) and
> using that, but I could cache the creds of whoever created each watch
> (ie. (2)) and pass that to post_one_notification() instead.
>
> However, it should be noted that (1) is the creds of the buffer owner.

How are buffers shared? Who besides the buffer creator can use it?

>>>  (e) All the points at which we walk over an object in a chain from (c) to
>>>      find the watch on which we can effect (d) (eg. we walk rootwards from a
>>>      mountpoint to find watches on a branch in the mount topology).
>> Smack does not require anything beyond existing checks.
> I'm glad to hear that, as this might be sufficiently impractical as to render
> it unusable with Smack.  Calling i_op->permissions() a lot would suck.
>
>>>  (y) What checks should be done on object destruction after final put and
>>>      what contexts need to be supplied?
>> Classically there is no such thing as filesystem object deletion.
>> By making it possible to set a watch on that you've inadvertently
>> added a security relevant action to the security model. :o
> That wasn't my original intention - I intended fsmount(2) to mount directly as
> mount(2) does, but Al had other ideas.
>
> Now fsmount(2) produces a file descriptor referring to a new mount object that
> can be mounted into by move_mount(2) before being spliced into the mount
> topology by move_mount(2).  However, if the fd is closed before the last step,
> close() will destroy the mount subtree attached to it (kind of quasi-unmount).
>
> That wouldn't be a problem, except that the fd from fsmount(2) can be used
> anywhere an O_PATH fd can be used - including watch_mount(2), fchdir(2), ...
> Further, FMODE_NEED_UNMOUNT gets cleared if the mount is spliced in at least
> once.
>
> Okay, having tried it you don't get an unmount event (since there's no actual
> unmount), but you do get an event to say that your watch got deleted (if the
> directory on which the watch was placed got removed from the system).
>
> So...  does the "your watch got deleted" message need checking?  In my
> opinion, it shouldn't get discarded because then the watcher doesn't know
> their watch went away.

Can you glean information from the watch being deleted?
I wouldn't think so, and it seems like a one-time event
from the system, so I don't think an access check would
be required.

>
> David