On Thu, Jun 6, 2019 at 9:43 AM Casey Schaufler <[email protected]> wrote: > > On 6/6/2019 7:05 AM, Stephen Smalley wrote: > > On 6/6/19 9:16 AM, David Howells wrote: > >> Stephen Smalley <[email protected]> wrote: > >> > >> This might be easier to discuss if you can reply to: > >> > >> https://lore.kernel.org/lkml/[email protected]/ > >> > >> 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. > >> > >>>> 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. > > > >>> 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? > > > > 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. > > > > >> 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. > > > >> 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.
