Re: [PATCH v4 2/4] fanotify: notify on mount attach and detach

[Daniel Burgener <dburgener@linux.microsoft.com>]

> If I understand the commit description correctly,
> security_path_notify(path, mask, FSNOTIFY_OBJ_TYPE_MNTNS) indicates a
> change in the mount namespace indicated by the @path parameter, with
> the initial mntns changes being limited to attach/detach and possibly
> some other attributes (see patch 4/4), although the latter looks like
> it will probably happen at a later date.
> 
> My initial thinking is that if we limit ourselves to existing SELinux
> policy permissions, this is much more of FILE__WATCH_MOUNT operation
> rather than a FILE__WATCH operation as while the /proc/PID/ns/mnt file
> specified in @path is simply a file, it represents much more than
> that.  However, it we want to consider adding a new SELinux policy
> permission (which is easy to do), we may want to consider adding a new
> mount namespace specific permission, e.g. FILE__WATCH_MOUNTNS, this
> would make it easier for policy developers to distinguish between
> watching a traditional mount point and a mount namespace (although
> given the common approaches to labeling this may not be very
> significant).  I'd personally like to hear from the SELinux policy
> folks on this (the SELinux reference policy has also been CC'd).
> 
> If we reuse the file/watch_mount permission the policy rule would look
> something like below where <subject> is the SELinux domain of the
> process making the change, and <mntns_label> is the label of the
> /proc/PID/ns/mnt file:
> 
>    allow <subject> <mntns_label>:file { watch_mount };
> 
> If we add a new file/watch_mountns permission the policy rule would
> look like this:
> 
>    allow <subject> <mntns_label>:file { watch_mountns };
> 

I've gone back and forth on this a few times.  If I understand it 
correctly, I think we might really want to have a new permission here, 
which is sad, because in my humble opinion, the watch_* permissions are 
already more complicated than I like.

"watch" does seem to be the wrong thing because this grants more than 
just changes to the specific file.  However, "watch_mount" is a very 
highly privileged operation.  Allowing watch on all reads and writes in 
the whole file hierarchy from a mount point is a substantial amount of 
access, and seems quite a bit more substantial than just watching new 
mounts being attached and detached (and similar) within a given mount 
namespace.

FWIW I do think the assumption that different labeling between /proc/pid 
files and mountpoints generally does make this not a problem in 
practice.  But in my opinion overloading watch_mount for this case seems 
different from the existing watch_mount permission to warrant not doing 
it.  Particularly with watch_mount being such a privileged operation.

-Daniel