Re: [RFC][PATCH 00/13] Mount, FS, Block and Keyrings notifications [ver #4]

From: Andy Lutomirski
Date: Mon Jun 10 2019 - 14:26:43 EST

> On Jun 10, 2019, at 11:01 AM, Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>> On 6/10/2019 9:42 AM, Andy Lutomirski wrote:
>>> On Mon, Jun 10, 2019 at 9:34 AM Casey Schaufler <casey@xxxxxxxxxxxxxxxx> wrote:
>>>> On 6/10/2019 8:21 AM, Stephen Smalley wrote:
>>>>> On 6/7/19 10:17 AM, David Howells wrote:
>>>>> Hi Al,
>>>>> Here's a set of patches to add a general variable-length notification queue
>>>>> concept and to add sources of events for:
>>>>> (1) Mount topology events, such as mounting, unmounting, mount expiry,
>>>>> mount reconfiguration.
>>>>> (2) Superblock events, such as R/W<->R/O changes, quota overrun and I/O
>>>>> errors (not complete yet).
>>>>> (3) Key/keyring events, such as creating, linking and removal of keys.
>>>>> (4) General device events (single common queue) including:
>>>>> - Block layer events, such as device errors
>>>>> - USB subsystem events, such as device/bus attach/remove, device
>>>>> reset, device errors.
>>>>> One of the reasons for this is so that we can remove the issue of processes
>>>>> having to repeatedly and regularly scan /proc/mounts, which has proven to
>>>>> be a system performance problem. To further aid this, the fsinfo() syscall
>>>>> on which this patch series depends, provides a way to access superblock and
>>>>> mount information in binary form without the need to parse /proc/mounts.
>>>>> 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 remain unconvinced that (1), (2), (3), and the final (?) above are a good idea.
>>>> For SELinux, I would expect that one would implement a collection of per watch-type WATCH permission checks on the target object (or to some well-defined object label like the kernel SID if there is no object) that allow receipt of all notifications of that watch-type for objects related to the target object, where "related to" is defined per watch-type.
>>>> I wouldn't expect SELinux to implement security_post_notification() at all. I can't see how one can construct a meaningful, stable policy for it. I'd argue that the triggering process is not posting the notification; the kernel is posting the notification and the watcher has been authorized to receive it.
>>> I cannot agree. There is an explicit action by a subject that results
>>> in information being delivered to an object. Just like a signal or a
>>> UDP packet delivery. Smack handles this kind of thing just fine. The
>>> internal mechanism that results in the access is irrelevant from
>>> this viewpoint. I can understand how a mechanism like SELinux that
>>> works on finer granularity might view it differently.
>> I think you really need to give an example of a coherent policy that
>> needs this.
> I keep telling you, and you keep ignoring what I say.
>> As it stands, your analogy seems confusing.
> It's pretty simple. I have given both the abstract
> and examples.

You gave the /dev/null example, which is inapplicable to this patchset.

>> If someone
>> changes the system clock, we don't restrict who is allowed to be
>> notified (via, for example, TFD_TIMER_CANCEL_ON_SET) that the clock
>> was changed based on who changed the clock.
> That's right. The system clock is not an object that
> unprivileged processes can modify. In fact, it is not
> an object at all. If you care to look, you will see that
> Smack does nothing with the clock.

And this is different from the mount tree how?

>> Similarly, if someone
>> tries to receive a packet on a socket, we check whether they have the
>> right to receive on that socket (from the endpoint in question) and,
>> if the sender is local, whether the sender can send to that socket.
>> We do not check whether the sender can send to the receiver.
> Bzzzt! Smack sure does.

This seems dubious. Iâm still trying to get you to explain to a non-Smack person why this makes sense.

>> The signal example is inapplicable.
> From a modeling viewpoint the actions are identical.

This seems incorrect to me and, I think, to most everyone else reading this. Can you explain?

In SELinux-ese, when you write to a file, the subject is the writer and the object is the file. When you send a signal to a process, the object is the target process.