Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Mon, Aug 12, 2019 at 9:41 AM Jan Kara wrote: > On Sat 10-08-19 11:01:16, Paul Moore wrote: > > On August 10, 2019 6:05:27 AM Amir Goldstein wrote: > > > > Other than Casey's comments, and ACK, I'm not seeing much commentary > > on this patch so FS and LSM folks consider this your last chance - if > > I don't hear any objections by the end of this week I'll plan on > > merging this into selinux/next next week. > > >>> > > >>> Please consider it is summer time so people may be on vacation like I > > >>> was... > > >> > > >> This is one of the reasons why I was speaking to the mailing list and > > >> not a particular individual :) > > > > > > Jan is fsnotify maintainer, so I think you should wait for an explicit ACK > > > from Jan or just merge the hook definition and ask Jan to merge to > > > fsnotify security hooks. > > > > Aaron posted his first patch a month ago in the beginning of July and I > > don't recall seeing any comments from Jan on any of the patch revisions. > > I would feel much better with an ACK/Reviewed-by from Jan, or you - which > > is why I sent that email - but I'm not going to wait forever and I'd like > > to get this into -next soon so we can get some testing. > > Yeah, sorry for the delays. I'm aware of the patch but I was also on > vacation and pretty busy at work so Amir always beat me in commenting on > the patch and I didn't have much to add. Once Aaron fixes the latest > comments from Amir, I'll give the patch the final look and give my ack. That is prefect, thanks. -- paul moore www.paul-moore.com
Re: [Non-DoD Source] Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 8/12/19 9:41 AM, Jan Kara wrote: On Sat 10-08-19 11:01:16, Paul Moore wrote: On August 10, 2019 6:05:27 AM Amir Goldstein wrote: Other than Casey's comments, and ACK, I'm not seeing much commentary on this patch so FS and LSM folks consider this your last chance - if I don't hear any objections by the end of this week I'll plan on merging this into selinux/next next week. Please consider it is summer time so people may be on vacation like I was... This is one of the reasons why I was speaking to the mailing list and not a particular individual :) Jan is fsnotify maintainer, so I think you should wait for an explicit ACK from Jan or just merge the hook definition and ask Jan to merge to fsnotify security hooks. Aaron posted his first patch a month ago in the beginning of July and I don't recall seeing any comments from Jan on any of the patch revisions. I would feel much better with an ACK/Reviewed-by from Jan, or you - which is why I sent that email - but I'm not going to wait forever and I'd like to get this into -next soon so we can get some testing. Yeah, sorry for the delays. I'm aware of the patch but I was also on vacation and pretty busy at work so Amir always beat me in commenting on the patch and I didn't have much to add. Once Aaron fixes the latest comments from Amir, I'll give the patch the final look and give my ack. Honza I already re-spun the patch with the changes Amir and I agreed to. There was an email with the PATCH v2. It may have flown under the radar a bit, so just wanted to point that out. -- Aaron
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Sat 10-08-19 11:01:16, Paul Moore wrote: > On August 10, 2019 6:05:27 AM Amir Goldstein wrote: > > Other than Casey's comments, and ACK, I'm not seeing much commentary > on this patch so FS and LSM folks consider this your last chance - if > I don't hear any objections by the end of this week I'll plan on > merging this into selinux/next next week. > >>> > >>> Please consider it is summer time so people may be on vacation like I > >>> was... > >> > >> This is one of the reasons why I was speaking to the mailing list and > >> not a particular individual :) > > > > Jan is fsnotify maintainer, so I think you should wait for an explicit ACK > > from Jan or just merge the hook definition and ask Jan to merge to > > fsnotify security hooks. > > Aaron posted his first patch a month ago in the beginning of July and I > don't recall seeing any comments from Jan on any of the patch revisions. > I would feel much better with an ACK/Reviewed-by from Jan, or you - which > is why I sent that email - but I'm not going to wait forever and I'd like > to get this into -next soon so we can get some testing. Yeah, sorry for the delays. I'm aware of the patch but I was also on vacation and pretty busy at work so Amir always beat me in commenting on the patch and I didn't have much to add. Once Aaron fixes the latest comments from Amir, I'll give the patch the final look and give my ack. Honza -- Jan Kara SUSE Labs, CR
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On August 10, 2019 6:05:27 AM Amir Goldstein wrote: Other than Casey's comments, and ACK, I'm not seeing much commentary on this patch so FS and LSM folks consider this your last chance - if I don't hear any objections by the end of this week I'll plan on merging this into selinux/next next week. >>> >>> Please consider it is summer time so people may be on vacation like I was... >> >> This is one of the reasons why I was speaking to the mailing list and >> not a particular individual :) > > Jan is fsnotify maintainer, so I think you should wait for an explicit ACK > from Jan or just merge the hook definition and ask Jan to merge to > fsnotify security hooks. Aaron posted his first patch a month ago in the beginning of July and I don't recall seeing any comments from Jan on any of the patch revisions. I would feel much better with an ACK/Reviewed-by from Jan, or you - which is why I sent that email - but I'm not going to wait forever and I'd like to get this into -next soon so we can get some testing. -- paul moore www.paul-moore.com
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
> > > Other than Casey's comments, and ACK, I'm not seeing much commentary > > > on this patch so FS and LSM folks consider this your last chance - if > > > I don't hear any objections by the end of this week I'll plan on > > > merging this into selinux/next next week. > > > > Please consider it is summer time so people may be on vacation like I was... > > This is one of the reasons why I was speaking to the mailing list and > not a particular individual :) > Jan is fsnotify maintainer, so I think you should wait for an explicit ACK from Jan or just merge the hook definition and ask Jan to merge to fsnotify security hooks. Thanks, Amir.
Re: [Non-DoD Source] Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
> >>> + switch (flags & FANOTIFY_MARK_TYPE_BITS) {
> >>> + case FAN_MARK_MOUNT:
> >>> + obj_type = FSNOTIFY_OBJ_TYPE_VFSMOUNT;
> >>> + break;
> >>> + case FAN_MARK_FILESYSTEM:
> >>> + obj_type = FSNOTIFY_OBJ_TYPE_SB;
> >>> + break;
> >>> + case FAN_MARK_INODE:
> >>> + obj_type = FSNOTIFY_OBJ_TYPE_INODE;
> >>> + break;
> >>> + default:
> >>> + ret = -EINVAL;
> >>> + goto out;
> >>> + }
> >
> > Sorry, I just can't stand this extra switch statement here.
> > Please initialize obj_type at the very first switch statement in
> > do_fanotify_mark() and pass it to fanotify_find_path().
> > Preferably also make it a helper that returns either
> > valid obj_type or <0 for error.
> >
> >
> I have no problem moving the initialization of obj_type up one level to
> do_fanotify_mark(). I don't think that a helper is necessary at this
> juncture as this logic seems to only exist in one place. Should this
> change, then there would be merit to having a separate function.
Ok.
> >>> +
> >>> + ret = security_path_notify(path, mask, obj_type);
> >>> if (ret)
> >>> path_put(path);
> >
> > It is probably best to mask out FANOTIFY_EVENT_FLAGS
> > when calling the hook. Although FAN_EVENT_ON_CHILD
> > and FAN_ONDIR do map to corresponding FS_ constants,
> > the security hooks from dnotify and inotify do not pass these
> > flags, so the security module cannot use them as reliable
> > information, so it will have to assume that they have been
> > requested anyway.
> >
> > Alternatively, make sure that dnotify/inotify security hooks
> > always set these two flags, by fixing up and using the
> > dnotify/inotify arg_to_mask conversion helpers before calling
> > the security hook.
> >
> I think that at this point either approach you mentioned makes just as
> much sense. If it's all the same to you, Amir, I'll just change the
> caller in fanotify to include (mask) & ~(FANOTIFY_EVENT_FLAGS)
On second look, let's go with (mask & ALL_FSNOTIFY_EVENTS)
It seems simpler and more appropriate way to convert to FS_ flags.
[...]
> >>>
> >>> - ret = inotify_find_inode(pathname, &path, flags);
> >>> + ret = inotify_find_inode(pathname, &path, flags, mask);
> >
> > Please use (mask & IN_ALL_EVENTS) for converting to common FS_ flags
> > or use the inotify_arg_to_mask() conversion helper, which contains more
> > details irrelevant for the security hook.
> > Otherwise mask may contain flags like IN_MASK_CREATE, which mean
> > different things on different backends and the security module cannot tell
> > the difference.
> >
> > Also note that at this point, before inotify_arg_to_mask(), the mask does
> > not yet contain FS_EVENT_ON_CHILD, which could be interesting for
> > the security hook (fanotify users can opt-in with FAN_EVENT_ON_CHILD).
> > Not a big deal though as security hook can assume the worse
> > (that events on child are requested).
> >
> I'll use (mask & IN_ALL_EVENTS).
OK.
>
> I can implement the changes in the ways I mentioned above. I don't see a
> need for anything more in the cases you brought up since none of them
> change the logic of the hook itself or would make a substantive
> difference to the operation of the hook given its current implementation.
>
Agree. If more flags are needed for LSMs they could be added later.
Thanks,
Amir.
Re: [Non-DoD Source] Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
... > >> First a suggestion, take it or leave it. > >> The name of the hook _notify() seems misleading to me. > >> naming the hook security_path_watch() seems much more > >> appropriate and matching the name of the constants FILE__WATCH > >> used by selinux. > > > > I guess I'm not too bothered by either name, Aaron? FWIW, if I was > > writing this hook, I would probably name it > > security_fsnotify_path(...). > > Or even just security_fsnotify() > > While I'm not necessarily attached to the name, I feel as though > "misleading" is too strong a word here. Agree. It is not misleading, but I should note that you yourself named the security class "watch", so why the inconsistency? > Notify seems to be an > appropriate enough term to me as every call to the hook, and thus all > the logic to which the hook adds security, lives in the notify/ subtree. > Well, if nobody cares about the name, it's fine by me. I wanted to point your attention to this proposal by David Howells: https://lore.kernel.org/linux-fsdevel/155991706847.15579.4702772917586301113.st...@warthog.procyon.org.uk/ His proposal adds new types of watches, for keyring changes, mount changes, etc and he proposed security hooks for setting new watches named "watch_XXX" and for posting notifications called "post_notification". The latter was later rejected by Stephen Smalley: https://lore.kernel.org/linux-fsdevel/cd657aab-e11c-c0b1-2e36-dd796ca75...@tycho.nsa.gov/ https://lore.kernel.org/linux-fsdevel/541e5cb3-142b-fe87-dff6-260b46d34...@tycho.nsa.gov/ Just to have a perspective why the hook name "notify_path" may end up being a bit ambiguous down the road. Thanks, Amir.
Re: [Non-DoD Source] Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 8/9/19 5:06 AM, Amir Goldstein wrote: On Thu, Aug 8, 2019 at 9:33 PM Paul Moore wrote: On Wed, Jul 31, 2019 at 11:35 AM Aaron Goidel wrote: As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient for the needs of SELinux. This is due to the fact that read access should not necessarily imply access to information about when another process reads from a file. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. This causes the issue of the watching application having the ability to exercise control over the triggering process. Without drawing a distinction within the permission check, the ability to read would imply the greater ability to control an application. Additionally, mount and superblock watches apply to all files within the same mount or superblock. Read access to one file should not necessarily imply the ability to watch all files accessed within a given mount or superblock. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target path has been resolved and are provided with the path struct, the mask of requested notification events, and the type of object on which the mark is being set (inode, superblock, or mount). The mask and obj_type have already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. The path struct is passed rather than just the inode so that the mount is available, particularly for mount watches. This also allows for use of the hook by pathname-based security modules. However, since the hook is intended for use even by inode based security modules, it is not placed under the CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security modules would need to enable all of the path hooks, even though they do not use any of them. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Security modules wishing to provide complete control over fanotify must also implement a security_file_open hook that validates that the access requested by the watching application is authorized. Fanotify has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. Otherwise, a watching process can obtain escalated access to a file for which it has not been authorized. The selinux_path_notify hook implementation works by adding five new file permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm (descriptions about which will follow), and one new filesystem permission: watch (which is applied to superblock checks). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask and the obj_type. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch, watch_mount, and watch_sb permissions are the baseline permissions for setting a watch on an object and each are a requirement for any watch to be set on a file, mount, or superblock respectively. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch, watch_mount, or watch_sb permission. Superblock watches further require the filesystem watch per
Re: [Non-DoD Source] Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 8/9/19 8:55 AM, Paul Moore wrote: On Fri, Aug 9, 2019 at 5:06 AM Amir Goldstein wrote: On Thu, Aug 8, 2019 at 9:33 PM Paul Moore wrote: On Wed, Jul 31, 2019 at 11:35 AM Aaron Goidel wrote: As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient for the needs of SELinux. This is due to the fact that read access should not necessarily imply access to information about when another process reads from a file. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. This causes the issue of the watching application having the ability to exercise control over the triggering process. Without drawing a distinction within the permission check, the ability to read would imply the greater ability to control an application. Additionally, mount and superblock watches apply to all files within the same mount or superblock. Read access to one file should not necessarily imply the ability to watch all files accessed within a given mount or superblock. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target path has been resolved and are provided with the path struct, the mask of requested notification events, and the type of object on which the mark is being set (inode, superblock, or mount). The mask and obj_type have already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. The path struct is passed rather than just the inode so that the mount is available, particularly for mount watches. This also allows for use of the hook by pathname-based security modules. However, since the hook is intended for use even by inode based security modules, it is not placed under the CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security modules would need to enable all of the path hooks, even though they do not use any of them. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Security modules wishing to provide complete control over fanotify must also implement a security_file_open hook that validates that the access requested by the watching application is authorized. Fanotify has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. Otherwise, a watching process can obtain escalated access to a file for which it has not been authorized. The selinux_path_notify hook implementation works by adding five new file permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm (descriptions about which will follow), and one new filesystem permission: watch (which is applied to superblock checks). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask and the obj_type. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch, watch_mount, and watch_sb permissions are the baseline permissions for setting a watch on an object and each are a requirement for any watch to be set on a file, mount, or superblock respectively. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch, watch_mount, or watch_sb permission. Superbloc
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Fri, Aug 9, 2019 at 5:06 AM Amir Goldstein wrote: > On Thu, Aug 8, 2019 at 9:33 PM Paul Moore wrote: > > On Wed, Jul 31, 2019 at 11:35 AM Aaron Goidel wrote: > > > As of now, setting watches on filesystem objects has, at most, applied a > > > check for read access to the inode, and in the case of fanotify, requires > > > CAP_SYS_ADMIN. No specific security hook or permission check has been > > > provided to control the setting of watches. Using any of inotify, dnotify, > > > or fanotify, it is possible to observe, not only write-like operations, > > > but > > > even read access to a file. Modeling the watch as being merely a read from > > > the file is insufficient for the needs of SELinux. This is due to the fact > > > that read access should not necessarily imply access to information about > > > when another process reads from a file. Furthermore, fanotify watches > > > grant > > > more power to an application in the form of permission events. While > > > notification events are solely, unidirectional (i.e. they only pass > > > information to the receiving application), permission events are blocking. > > > Permission events make a request to the receiving application which will > > > then reply with a decision as to whether or not that action may be > > > completed. This causes the issue of the watching application having the > > > ability to exercise control over the triggering process. Without drawing a > > > distinction within the permission check, the ability to read would imply > > > the greater ability to control an application. Additionally, mount and > > > superblock watches apply to all files within the same mount or superblock. > > > Read access to one file should not necessarily imply the ability to watch > > > all files accessed within a given mount or superblock. > > > > > > In order to solve these issues, a new LSM hook is implemented and has been > > > placed within the system calls for marking filesystem objects with > > > inotify, > > > fanotify, and dnotify watches. These calls to the hook are placed at the > > > point at which the target path has been resolved and are provided with the > > > path struct, the mask of requested notification events, and the type of > > > object on which the mark is being set (inode, superblock, or mount). The > > > mask and obj_type have already been translated into common FS_* values > > > shared by the entirety of the fs notification infrastructure. The path > > > struct is passed rather than just the inode so that the mount is > > > available, > > > particularly for mount watches. This also allows for use of the hook by > > > pathname-based security modules. However, since the hook is intended for > > > use even by inode based security modules, it is not placed under the > > > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security > > > modules would need to enable all of the path hooks, even though they do > > > not > > > use any of them. > > > > > > This only provides a hook at the point of setting a watch, and presumes > > > that permission to set a particular watch implies the ability to receive > > > all notification about that object which match the mask. This is all that > > > is required for SELinux. If other security modules require additional > > > hooks > > > or infrastructure to control delivery of notification, these can be added > > > by them. It does not make sense for us to propose hooks for which we have > > > no implementation. The understanding that all notifications received by > > > the > > > requesting application are all strictly of a type for which the > > > application > > > has been granted permission shows that this implementation is sufficient > > > in > > > its coverage. > > > > > > Security modules wishing to provide complete control over fanotify must > > > also implement a security_file_open hook that validates that the access > > > requested by the watching application is authorized. Fanotify has the > > > issue > > > that it returns a file descriptor with the file mode specified during > > > fanotify_init() to the watching process on event. This is already covered > > > by the LSM security_file_open hook if the security module implements > > > checking of the requested file mode there. Otherwise, a watching process > > > can obtain escalated access to a file for which it has not been > > > authorized. > > > > > > The selinux_path_notify hook implementation works by adding five new file > > > permissions: watch, watch_mount, watch_sb, watch_reads, and > > > watch_with_perm > > > (descriptions about which will follow), and one new filesystem permission: > > > watch (which is applied to superblock checks). The hook then decides which > > > subset of these permissions must be held by the requesting application > > > based on the contents of the provided mask and the obj_type. The > > > selinux_file_open hook already checks the requested file mode and > > > therefore > > > ensures that a
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Thu, Aug 8, 2019 at 9:33 PM Paul Moore wrote: > > On Wed, Jul 31, 2019 at 11:35 AM Aaron Goidel wrote: > > As of now, setting watches on filesystem objects has, at most, applied a > > check for read access to the inode, and in the case of fanotify, requires > > CAP_SYS_ADMIN. No specific security hook or permission check has been > > provided to control the setting of watches. Using any of inotify, dnotify, > > or fanotify, it is possible to observe, not only write-like operations, but > > even read access to a file. Modeling the watch as being merely a read from > > the file is insufficient for the needs of SELinux. This is due to the fact > > that read access should not necessarily imply access to information about > > when another process reads from a file. Furthermore, fanotify watches grant > > more power to an application in the form of permission events. While > > notification events are solely, unidirectional (i.e. they only pass > > information to the receiving application), permission events are blocking. > > Permission events make a request to the receiving application which will > > then reply with a decision as to whether or not that action may be > > completed. This causes the issue of the watching application having the > > ability to exercise control over the triggering process. Without drawing a > > distinction within the permission check, the ability to read would imply > > the greater ability to control an application. Additionally, mount and > > superblock watches apply to all files within the same mount or superblock. > > Read access to one file should not necessarily imply the ability to watch > > all files accessed within a given mount or superblock. > > > > In order to solve these issues, a new LSM hook is implemented and has been > > placed within the system calls for marking filesystem objects with inotify, > > fanotify, and dnotify watches. These calls to the hook are placed at the > > point at which the target path has been resolved and are provided with the > > path struct, the mask of requested notification events, and the type of > > object on which the mark is being set (inode, superblock, or mount). The > > mask and obj_type have already been translated into common FS_* values > > shared by the entirety of the fs notification infrastructure. The path > > struct is passed rather than just the inode so that the mount is available, > > particularly for mount watches. This also allows for use of the hook by > > pathname-based security modules. However, since the hook is intended for > > use even by inode based security modules, it is not placed under the > > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security > > modules would need to enable all of the path hooks, even though they do not > > use any of them. > > > > This only provides a hook at the point of setting a watch, and presumes > > that permission to set a particular watch implies the ability to receive > > all notification about that object which match the mask. This is all that > > is required for SELinux. If other security modules require additional hooks > > or infrastructure to control delivery of notification, these can be added > > by them. It does not make sense for us to propose hooks for which we have > > no implementation. The understanding that all notifications received by the > > requesting application are all strictly of a type for which the application > > has been granted permission shows that this implementation is sufficient in > > its coverage. > > > > Security modules wishing to provide complete control over fanotify must > > also implement a security_file_open hook that validates that the access > > requested by the watching application is authorized. Fanotify has the issue > > that it returns a file descriptor with the file mode specified during > > fanotify_init() to the watching process on event. This is already covered > > by the LSM security_file_open hook if the security module implements > > checking of the requested file mode there. Otherwise, a watching process > > can obtain escalated access to a file for which it has not been authorized. > > > > The selinux_path_notify hook implementation works by adding five new file > > permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm > > (descriptions about which will follow), and one new filesystem permission: > > watch (which is applied to superblock checks). The hook then decides which > > subset of these permissions must be held by the requesting application > > based on the contents of the provided mask and the obj_type. The > > selinux_file_open hook already checks the requested file mode and therefore > > ensures that a watching process cannot escalate its access through > > fanotify. > > > > The watch, watch_mount, and watch_sb permissions are the baseline > > permissions for setting a watch on an object and each are a requirement for > > any watch to be set on a file, mount, or superblock res
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, Jul 31, 2019 at 11:35 AM Aaron Goidel wrote: > As of now, setting watches on filesystem objects has, at most, applied a > check for read access to the inode, and in the case of fanotify, requires > CAP_SYS_ADMIN. No specific security hook or permission check has been > provided to control the setting of watches. Using any of inotify, dnotify, > or fanotify, it is possible to observe, not only write-like operations, but > even read access to a file. Modeling the watch as being merely a read from > the file is insufficient for the needs of SELinux. This is due to the fact > that read access should not necessarily imply access to information about > when another process reads from a file. Furthermore, fanotify watches grant > more power to an application in the form of permission events. While > notification events are solely, unidirectional (i.e. they only pass > information to the receiving application), permission events are blocking. > Permission events make a request to the receiving application which will > then reply with a decision as to whether or not that action may be > completed. This causes the issue of the watching application having the > ability to exercise control over the triggering process. Without drawing a > distinction within the permission check, the ability to read would imply > the greater ability to control an application. Additionally, mount and > superblock watches apply to all files within the same mount or superblock. > Read access to one file should not necessarily imply the ability to watch > all files accessed within a given mount or superblock. > > In order to solve these issues, a new LSM hook is implemented and has been > placed within the system calls for marking filesystem objects with inotify, > fanotify, and dnotify watches. These calls to the hook are placed at the > point at which the target path has been resolved and are provided with the > path struct, the mask of requested notification events, and the type of > object on which the mark is being set (inode, superblock, or mount). The > mask and obj_type have already been translated into common FS_* values > shared by the entirety of the fs notification infrastructure. The path > struct is passed rather than just the inode so that the mount is available, > particularly for mount watches. This also allows for use of the hook by > pathname-based security modules. However, since the hook is intended for > use even by inode based security modules, it is not placed under the > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security > modules would need to enable all of the path hooks, even though they do not > use any of them. > > This only provides a hook at the point of setting a watch, and presumes > that permission to set a particular watch implies the ability to receive > all notification about that object which match the mask. This is all that > is required for SELinux. If other security modules require additional hooks > or infrastructure to control delivery of notification, these can be added > by them. It does not make sense for us to propose hooks for which we have > no implementation. The understanding that all notifications received by the > requesting application are all strictly of a type for which the application > has been granted permission shows that this implementation is sufficient in > its coverage. > > Security modules wishing to provide complete control over fanotify must > also implement a security_file_open hook that validates that the access > requested by the watching application is authorized. Fanotify has the issue > that it returns a file descriptor with the file mode specified during > fanotify_init() to the watching process on event. This is already covered > by the LSM security_file_open hook if the security module implements > checking of the requested file mode there. Otherwise, a watching process > can obtain escalated access to a file for which it has not been authorized. > > The selinux_path_notify hook implementation works by adding five new file > permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm > (descriptions about which will follow), and one new filesystem permission: > watch (which is applied to superblock checks). The hook then decides which > subset of these permissions must be held by the requesting application > based on the contents of the provided mask and the obj_type. The > selinux_file_open hook already checks the requested file mode and therefore > ensures that a watching process cannot escalate its access through > fanotify. > > The watch, watch_mount, and watch_sb permissions are the baseline > permissions for setting a watch on an object and each are a requirement for > any watch to be set on a file, mount, or superblock respectively. It should > be noted that having either of the other two permissions (watch_reads and > watch_with_perm) does not imply the watch, watch_mount, or watch_sb > permission. Superblock watc
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Thu, Aug 1, 2019 at 7:31 AM Stephen Smalley wrote:
> On 7/31/19 8:27 PM, Paul Moore wrote:
> > On Wed, Jul 31, 2019 at 1:26 PM Casey Schaufler
> > wrote:
> >> On 7/31/2019 8:34 AM, Aaron Goidel wrote:
...
> >>> +static int selinux_path_notify(const struct path *path, u64 mask,
> >>> + unsigned int obj_type)
> >>> +{
> >>> + int ret;
> >>> + u32 perm;
> >>> +
> >>> + struct common_audit_data ad;
> >>> +
> >>> + ad.type = LSM_AUDIT_DATA_PATH;
> >>> + ad.u.path = *path;
> >>> +
> >>> + /*
> >>> + * Set permission needed based on the type of mark being set.
> >>> + * Performs an additional check for sb watches.
> >>> + */
> >>> + switch (obj_type) {
> >>> + case FSNOTIFY_OBJ_TYPE_VFSMOUNT:
> >>> + perm = FILE__WATCH_MOUNT;
> >>> + break;
> >>> + case FSNOTIFY_OBJ_TYPE_SB:
> >>> + perm = FILE__WATCH_SB;
> >>> + ret = superblock_has_perm(current_cred(),
> >>> path->dentry->d_sb,
> >>> + FILESYSTEM__WATCH, &ad);
> >>> + if (ret)
> >>> + return ret;
> >>> + break;
> >>> + case FSNOTIFY_OBJ_TYPE_INODE:
> >>> + perm = FILE__WATCH;
> >>> + break;
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + // check if the mask is requesting ability to set a blocking watch
> >
> > ... in the future please don't use "// XXX", use "/* XXX */" instead :)
> >
> > Don't respin the patch just for this, but if you have to do it for
> > some other reason please fix the C++ style comments. Thanks.
>
> This was discussed during the earlier RFC series but ultimately someone
> pointed to:
> https://lkml.org/lkml/2016/7/8/625
> where Linus blessed the use of C++/C99 style comments. And checkpatch
> accepts them these days.
Yep, I'm aware of both, it is simply a personal preference of mine.
I'm not going to reject patches with C++ style comments, but I would
ask people to stick to the good ol' fashioned comments for patches
they submit.
> Obviously if you truly don't want them in the SELinux code, that's your
> call. But note that all files now have at least one such comment as a
> result of the mass SPDX license headers that were added throughout the
> tree using that style.
FYI, the sky is blue.
It isn't just the license headers either, Al dropped one into hooks.c
:). Just like I don't plan to reject patches due only to the comment
style, you don't see me pushing patches to change the C++ comments.
--
paul moore
www.paul-moore.com
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/31/19 8:27 PM, Paul Moore wrote: On Wed, Jul 31, 2019 at 1:26 PM Casey Schaufler wrote: On 7/31/2019 8:34 AM, Aaron Goidel wrote: As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient for the needs of SELinux. This is due to the fact that read access should not necessarily imply access to information about when another process reads from a file. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. This causes the issue of the watching application having the ability to exercise control over the triggering process. Without drawing a distinction within the permission check, the ability to read would imply the greater ability to control an application. Additionally, mount and superblock watches apply to all files within the same mount or superblock. Read access to one file should not necessarily imply the ability to watch all files accessed within a given mount or superblock. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target path has been resolved and are provided with the path struct, the mask of requested notification events, and the type of object on which the mark is being set (inode, superblock, or mount). The mask and obj_type have already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. The path struct is passed rather than just the inode so that the mount is available, particularly for mount watches. This also allows for use of the hook by pathname-based security modules. However, since the hook is intended for use even by inode based security modules, it is not placed under the CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security modules would need to enable all of the path hooks, even though they do not use any of them. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Security modules wishing to provide complete control over fanotify must also implement a security_file_open hook that validates that the access requested by the watching application is authorized. Fanotify has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. Otherwise, a watching process can obtain escalated access to a file for which it has not been authorized. The selinux_path_notify hook implementation works by adding five new file permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm (descriptions about which will follow), and one new filesystem permission: watch (which is applied to superblock checks). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask and the obj_type. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch, watch_mount, and watch_sb permissions are the baseline permissions for setting a watch on an object and each are a requirement for any watch to be set on a file, mount, or superblock respectively. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch, watch_mount, or watch_sb permission. Superblock watches further require the filesystem watch permission to th
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, Jul 31, 2019 at 1:26 PM Casey Schaufler wrote: > On 7/31/2019 8:34 AM, Aaron Goidel wrote: > > As of now, setting watches on filesystem objects has, at most, applied a > > check for read access to the inode, and in the case of fanotify, requires > > CAP_SYS_ADMIN. No specific security hook or permission check has been > > provided to control the setting of watches. Using any of inotify, dnotify, > > or fanotify, it is possible to observe, not only write-like operations, but > > even read access to a file. Modeling the watch as being merely a read from > > the file is insufficient for the needs of SELinux. This is due to the fact > > that read access should not necessarily imply access to information about > > when another process reads from a file. Furthermore, fanotify watches grant > > more power to an application in the form of permission events. While > > notification events are solely, unidirectional (i.e. they only pass > > information to the receiving application), permission events are blocking. > > Permission events make a request to the receiving application which will > > then reply with a decision as to whether or not that action may be > > completed. This causes the issue of the watching application having the > > ability to exercise control over the triggering process. Without drawing a > > distinction within the permission check, the ability to read would imply > > the greater ability to control an application. Additionally, mount and > > superblock watches apply to all files within the same mount or superblock. > > Read access to one file should not necessarily imply the ability to watch > > all files accessed within a given mount or superblock. > > > > In order to solve these issues, a new LSM hook is implemented and has been > > placed within the system calls for marking filesystem objects with inotify, > > fanotify, and dnotify watches. These calls to the hook are placed at the > > point at which the target path has been resolved and are provided with the > > path struct, the mask of requested notification events, and the type of > > object on which the mark is being set (inode, superblock, or mount). The > > mask and obj_type have already been translated into common FS_* values > > shared by the entirety of the fs notification infrastructure. The path > > struct is passed rather than just the inode so that the mount is available, > > particularly for mount watches. This also allows for use of the hook by > > pathname-based security modules. However, since the hook is intended for > > use even by inode based security modules, it is not placed under the > > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security > > modules would need to enable all of the path hooks, even though they do not > > use any of them. > > > > This only provides a hook at the point of setting a watch, and presumes > > that permission to set a particular watch implies the ability to receive > > all notification about that object which match the mask. This is all that > > is required for SELinux. If other security modules require additional hooks > > or infrastructure to control delivery of notification, these can be added > > by them. It does not make sense for us to propose hooks for which we have > > no implementation. The understanding that all notifications received by the > > requesting application are all strictly of a type for which the application > > has been granted permission shows that this implementation is sufficient in > > its coverage. > > > > Security modules wishing to provide complete control over fanotify must > > also implement a security_file_open hook that validates that the access > > requested by the watching application is authorized. Fanotify has the issue > > that it returns a file descriptor with the file mode specified during > > fanotify_init() to the watching process on event. This is already covered > > by the LSM security_file_open hook if the security module implements > > checking of the requested file mode there. Otherwise, a watching process > > can obtain escalated access to a file for which it has not been authorized. > > > > The selinux_path_notify hook implementation works by adding five new file > > permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm > > (descriptions about which will follow), and one new filesystem permission: > > watch (which is applied to superblock checks). The hook then decides which > > subset of these permissions must be held by the requesting application > > based on the contents of the provided mask and the obj_type. The > > selinux_file_open hook already checks the requested file mode and therefore > > ensures that a watching process cannot escalate its access through > > fanotify. > > > > The watch, watch_mount, and watch_sb permissions are the baseline > > permissions for setting a watch on an object and each are a requirement for > > any watch to be set on a file, mount, or superblock respectivel
Re: [PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/31/2019 8:34 AM, Aaron Goidel wrote: > As of now, setting watches on filesystem objects has, at most, applied a > check for read access to the inode, and in the case of fanotify, requires > CAP_SYS_ADMIN. No specific security hook or permission check has been > provided to control the setting of watches. Using any of inotify, dnotify, > or fanotify, it is possible to observe, not only write-like operations, but > even read access to a file. Modeling the watch as being merely a read from > the file is insufficient for the needs of SELinux. This is due to the fact > that read access should not necessarily imply access to information about > when another process reads from a file. Furthermore, fanotify watches grant > more power to an application in the form of permission events. While > notification events are solely, unidirectional (i.e. they only pass > information to the receiving application), permission events are blocking. > Permission events make a request to the receiving application which will > then reply with a decision as to whether or not that action may be > completed. This causes the issue of the watching application having the > ability to exercise control over the triggering process. Without drawing a > distinction within the permission check, the ability to read would imply > the greater ability to control an application. Additionally, mount and > superblock watches apply to all files within the same mount or superblock. > Read access to one file should not necessarily imply the ability to watch > all files accessed within a given mount or superblock. > > In order to solve these issues, a new LSM hook is implemented and has been > placed within the system calls for marking filesystem objects with inotify, > fanotify, and dnotify watches. These calls to the hook are placed at the > point at which the target path has been resolved and are provided with the > path struct, the mask of requested notification events, and the type of > object on which the mark is being set (inode, superblock, or mount). The > mask and obj_type have already been translated into common FS_* values > shared by the entirety of the fs notification infrastructure. The path > struct is passed rather than just the inode so that the mount is available, > particularly for mount watches. This also allows for use of the hook by > pathname-based security modules. However, since the hook is intended for > use even by inode based security modules, it is not placed under the > CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security > modules would need to enable all of the path hooks, even though they do not > use any of them. > > This only provides a hook at the point of setting a watch, and presumes > that permission to set a particular watch implies the ability to receive > all notification about that object which match the mask. This is all that > is required for SELinux. If other security modules require additional hooks > or infrastructure to control delivery of notification, these can be added > by them. It does not make sense for us to propose hooks for which we have > no implementation. The understanding that all notifications received by the > requesting application are all strictly of a type for which the application > has been granted permission shows that this implementation is sufficient in > its coverage. > > Security modules wishing to provide complete control over fanotify must > also implement a security_file_open hook that validates that the access > requested by the watching application is authorized. Fanotify has the issue > that it returns a file descriptor with the file mode specified during > fanotify_init() to the watching process on event. This is already covered > by the LSM security_file_open hook if the security module implements > checking of the requested file mode there. Otherwise, a watching process > can obtain escalated access to a file for which it has not been authorized. > > The selinux_path_notify hook implementation works by adding five new file > permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm > (descriptions about which will follow), and one new filesystem permission: > watch (which is applied to superblock checks). The hook then decides which > subset of these permissions must be held by the requesting application > based on the contents of the provided mask and the obj_type. The > selinux_file_open hook already checks the requested file mode and therefore > ensures that a watching process cannot escalate its access through > fanotify. > > The watch, watch_mount, and watch_sb permissions are the baseline > permissions for setting a watch on an object and each are a requirement for > any watch to be set on a file, mount, or superblock respectively. It should > be noted that having either of the other two permissions (watch_reads and > watch_with_perm) does not imply the watch, watch_mount, or watch_sb > permission. Superblock watches further
[PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient for the needs of SELinux. This is due to the fact that read access should not necessarily imply access to information about when another process reads from a file. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. This causes the issue of the watching application having the ability to exercise control over the triggering process. Without drawing a distinction within the permission check, the ability to read would imply the greater ability to control an application. Additionally, mount and superblock watches apply to all files within the same mount or superblock. Read access to one file should not necessarily imply the ability to watch all files accessed within a given mount or superblock. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target path has been resolved and are provided with the path struct, the mask of requested notification events, and the type of object on which the mark is being set (inode, superblock, or mount). The mask and obj_type have already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. The path struct is passed rather than just the inode so that the mount is available, particularly for mount watches. This also allows for use of the hook by pathname-based security modules. However, since the hook is intended for use even by inode based security modules, it is not placed under the CONFIG_SECURITY_PATH conditional. Otherwise, the inode-based security modules would need to enable all of the path hooks, even though they do not use any of them. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Security modules wishing to provide complete control over fanotify must also implement a security_file_open hook that validates that the access requested by the watching application is authorized. Fanotify has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. Otherwise, a watching process can obtain escalated access to a file for which it has not been authorized. The selinux_path_notify hook implementation works by adding five new file permissions: watch, watch_mount, watch_sb, watch_reads, and watch_with_perm (descriptions about which will follow), and one new filesystem permission: watch (which is applied to superblock checks). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask and the obj_type. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch, watch_mount, and watch_sb permissions are the baseline permissions for setting a watch on an object and each are a requirement for any watch to be set on a file, mount, or superblock respectively. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch, watch_mount, or watch_sb permission. Superblock watches further require the filesystem watch permission to the superblock. As there is no labeled object in view for mounts, there is no specific check for mount watches beyond watch_mount to the inode
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, 10 Jul 2019, Casey Schaufler wrote: > On 7/10/2019 6:34 AM, Aaron Goidel wrote: > > > Furthermore, fanotify watches grant more power to > > an application in the form of permission events. While notification events > > are solely, unidirectional (i.e. they only pass information to the > > receiving application), permission events are blocking. Permission events > > make a request to the receiving application which will then reply with a > > decision as to whether or not that action may be completed. > > You're not saying why this is an issue. Also in the description, please explain the issues with read and write notifications and why a simple 'read' permission is not adequate. -- James Morris
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/2019 11:39 AM, Stephen Smalley wrote: > On 7/10/19 12:38 PM, Casey Schaufler wrote: >> On 7/10/2019 6:34 AM, Aaron Goidel wrote: >>> As of now, setting watches on filesystem objects has, at most, applied a >>> check for read access to the inode, and in the case of fanotify, requires >>> CAP_SYS_ADMIN. No specific security hook or permission check has been >>> provided to control the setting of watches. Using any of inotify, dnotify, >>> or fanotify, it is possible to observe, not only write-like operations, but >>> even read access to a file. Modeling the watch as being merely a read from >>> the file is insufficient. >> >> That's a very model-specific viewpoint. It is true for >> a fine-grained model such as SELinux, but not necessarily >> for a model with more traditional object definitions. >> I'm not saying you're wrong, I'm saying that stating it >> as a given assumes your model. You can do that all you want >> within SELinux, but it doesn't hold when you're talking >> about the LSM infrastructure. > > I think you'll find that even for Smack, merely checking read access to the > watched inode is insufficient for your purposes, because the watch permits > more than just observing changes to the state of the inode. The absence of a > hook is a gap in LSM coverage, regardless of security model. If you are just > objecting to the wording choice, then I suppose that can be amended to "is > insufficient for SELinux" or "is insufficient for some needs" or something. More an objection to the assumption of model than anything else. There are enough differing viewpoints on what is necessary and/or sufficient that I wouldn't want the assumption to be a bone of contention later on. > >> Have you coordinated this with the work that David Howells >> is doing on generic notifications? > > We're following that work but to date it hasn't appeared to address > dnotify/inotify/fanotify IIUC. I think it is complementary; we are adding > LSM control over an existing kernel notification mechanism while he is adding > a new notification facility for other kinds of events along with > corresponding LSM hooks. It is consistent in that it provides a way to > control setting of watches based on the watched object. All true. My hope is that LSM controls on notification mechanisms have some sort of coordination. I'd rather have one hook that's used in multiple places than yet another set of disparate hooks that do mostly the same thing. > >>> Furthermore, fanotify watches grant more power to >>> an application in the form of permission events. While notification events >>> are solely, unidirectional (i.e. they only pass information to the >>> receiving application), permission events are blocking. Permission events >>> make a request to the receiving application which will then reply with a >>> decision as to whether or not that action may be completed. >> >> You're not saying why this is an issue. > > It allows the watching application control over the process that is > attempting the access. Are you just asking for that to be stated more > explicitly? Yes, that would be good. > >>> In order to solve these issues, a new LSM hook is implemented and has been >>> placed within the system calls for marking filesystem objects with inotify, >>> fanotify, and dnotify watches. These calls to the hook are placed at the >>> point at which the target inode has been resolved and are provided with >>> both the inode and the mask of requested notification events. The mask has >>> already been translated into common FS_* values shared by the entirety of >>> the fs notification infrastructure. >>> >>> This only provides a hook at the point of setting a watch, and presumes >>> that permission to set a particular watch implies the ability to receive >>> all notification about that object which match the mask. This is all that >>> is required for SELinux. If other security modules require additional hooks >>> or infrastructure to control delivery of notification, these can be added >>> by them. It does not make sense for us to propose hooks for which we have >>> no implementation. The understanding that all notifications received by the >>> requesting application are all strictly of a type for which the application >>> has been granted permission shows that this implementation is sufficient in >>> its coverage. >> >> A reasonable approach. It would be *nice* if you had >> a look at the other security modules to see what they >> might need from such a hook or hook set. >> >>> Fanotify further has the issue that it returns a file descriptor with the >>> file mode specified during fanotify_init() to the watching process on >>> event. This is already covered by the LSM security_file_open hook if the >>> security module implements checking of the requested file mode there. >> >> How is this relevant? > > It is part of ensuring complete control over fanotify. Some existing > security modules (like Smack, for example) cur
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/19 12:38 PM, Casey Schaufler wrote: On 7/10/2019 6:34 AM, Aaron Goidel wrote: As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient. That's a very model-specific viewpoint. It is true for a fine-grained model such as SELinux, but not necessarily for a model with more traditional object definitions. I'm not saying you're wrong, I'm saying that stating it as a given assumes your model. You can do that all you want within SELinux, but it doesn't hold when you're talking about the LSM infrastructure. I think you'll find that even for Smack, merely checking read access to the watched inode is insufficient for your purposes, because the watch permits more than just observing changes to the state of the inode. The absence of a hook is a gap in LSM coverage, regardless of security model. If you are just objecting to the wording choice, then I suppose that can be amended to "is insufficient for SELinux" or "is insufficient for some needs" or something. Have you coordinated this with the work that David Howells is doing on generic notifications? We're following that work but to date it hasn't appeared to address dnotify/inotify/fanotify IIUC. I think it is complementary; we are adding LSM control over an existing kernel notification mechanism while he is adding a new notification facility for other kinds of events along with corresponding LSM hooks. It is consistent in that it provides a way to control setting of watches based on the watched object. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. You're not saying why this is an issue. It allows the watching application control over the process that is attempting the access. Are you just asking for that to be stated more explicitly? In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target inode has been resolved and are provided with both the inode and the mask of requested notification events. The mask has already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. A reasonable approach. It would be *nice* if you had a look at the other security modules to see what they might need from such a hook or hook set. Fanotify further has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. How is this relevant? It is part of ensuring complete control over fanotify. Some existing security modules (like Smack, for example) currently do not perform this checking of the requested file mode and therefore are subject to this privilege escalation scenario through fanotify. A watcher that only has read access to the file can get a read-write descriptor to it in this manner. You may argue that this doesn't matter because fanotify requires CAP_SYS_ADMIN but even for Smack that isn't the same as CAP_MAC_OVERRIDE. The selinux_inode_notify hook implementation works by adding three new file permissions: watch, watch_reads, and watch_with_perm (descriptions about which will follow). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask. The selinux_file_open ho
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/19 10:22 AM, Joe Perches wrote:
> On Wed, 2019-07-10 at 10:18 -0700, Joe Perches wrote:
>> On Wed, 2019-07-10 at 09:49 -0700, Randy Dunlap wrote:
>>> On 7/10/19 9:38 AM, Casey Schaufler wrote:
On 7/10/2019 6:34 AM, Aaron Goidel wrote:
> @@ -3261,6 +3262,26 @@ static int selinux_inode_removexattr(struct dentry
> *dentry, const char *name)
> return -EACCES;
> }
>
> +static int selinux_inode_notify(struct inode *inode, u64 mask)
> +{
> + u32 perm = FILE__WATCH; // basic permission, can a watch be set?
We don't use // comments in the Linux kernel.
>>>
>>> I thought that we had recently moved into the 21st century on that issue,
>>> but I don't see it mentioned in coding-style.rst. Maybe we need a Doc
>>> update.
>>>
>>> checkpatch allows C99 comments by default.
>>> Joe, do you recall about this?
>>
>> My recollection is it was something I thought was
>> just simple and useful so I added it to checkpatch
>> without going through the negative of the nominal
>> approvals required by modifying CodingStyle.
>
> https://lkml.org/lkml/2016/7/8/625
>
Aha, thanks, I don't recall seeing that one.
--
~Randy
Re: [Non-DoD Source] Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/19 10:55 AM, Amir Goldstein wrote: On Wed, Jul 10, 2019 at 4:34 PM Aaron Goidel wrote: As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target inode has been resolved and are provided with both the inode and the mask of requested notification events. The mask has already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Fanotify further has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. The selinux_inode_notify hook implementation works by adding three new file permissions: watch, watch_reads, and watch_with_perm (descriptions about which will follow). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch permission is the baseline permission for setting a watch on an object and is a requirement for any watch to be set whatsoever. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch permission, though this could be changed if need be. The watch_reads permission is required to receive notifications from read-exclusive events on filesystem objects. These events include accessing a file for the purpose of reading and closing a file which has been opened read-only. This distinction has been drawn in order to provide a direct indication in the policy for this otherwise not obvious capability. Read access to a file should not necessarily imply the ability to observe read events on a file. Finally, watch_with_perm only applies to fanotify masks since it is the only way to set a mask which allows for the blocking, permission event. This permission is needed for any watch which is of this type. Though fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit trust to root, which we do not do, and does not support least privilege. Signed-off-by: Aaron Goidel --- fs/notify/dnotify/dnotify.c | 14 +++--- fs/notify/fanotify/fanotify_user.c | 11 +-- fs/notify/inotify/inotify_user.c| 12 ++-- include/linux/lsm_hooks.h | 2 ++ include/linux/security.h| 7 +++ security/security.c | 5 + security/selinux/hooks.c| 22 ++ security/selinux/include/classmap.h | 2 +- 8 files changed, 67 insertions(+), 8 deletions(-) diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 250369d6901d..e91ce092efb1 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -288,6 +289,16 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg) goto out_err; } + /* +* convert the userspa
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, 2019-07-10 at 10:18 -0700, Joe Perches wrote:
> On Wed, 2019-07-10 at 09:49 -0700, Randy Dunlap wrote:
> > On 7/10/19 9:38 AM, Casey Schaufler wrote:
> > > On 7/10/2019 6:34 AM, Aaron Goidel wrote:
> > > > @@ -3261,6 +3262,26 @@ static int selinux_inode_removexattr(struct
> > > > dentry *dentry, const char *name)
> > > > return -EACCES;
> > > > }
> > > >
> > > > +static int selinux_inode_notify(struct inode *inode, u64 mask)
> > > > +{
> > > > + u32 perm = FILE__WATCH; // basic permission, can a watch be set?
> > >
> > > We don't use // comments in the Linux kernel.
> > >
> >
> > I thought that we had recently moved into the 21st century on that issue,
> > but I don't see it mentioned in coding-style.rst. Maybe we need a Doc
> > update.
> >
> > checkpatch allows C99 comments by default.
> > Joe, do you recall about this?
>
> My recollection is it was something I thought was
> just simple and useful so I added it to checkpatch
> without going through the negative of the nominal
> approvals required by modifying CodingStyle.
https://lkml.org/lkml/2016/7/8/625
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, 2019-07-10 at 09:49 -0700, Randy Dunlap wrote:
> On 7/10/19 9:38 AM, Casey Schaufler wrote:
> > On 7/10/2019 6:34 AM, Aaron Goidel wrote:
> > > @@ -3261,6 +3262,26 @@ static int selinux_inode_removexattr(struct dentry
> > > *dentry, const char *name)
> > > return -EACCES;
> > > }
> > >
> > > +static int selinux_inode_notify(struct inode *inode, u64 mask)
> > > +{
> > > + u32 perm = FILE__WATCH; // basic permission, can a watch be set?
> >
> > We don't use // comments in the Linux kernel.
> >
>
> I thought that we had recently moved into the 21st century on that issue,
> but I don't see it mentioned in coding-style.rst. Maybe we need a Doc update.
>
> checkpatch allows C99 comments by default.
> Joe, do you recall about this?
My recollection is it was something I thought was
just simple and useful so I added it to checkpatch
without going through the negative of the nominal
approvals required by modifying CodingStyle.
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/2019 9:49 AM, Randy Dunlap wrote:
> On 7/10/19 9:38 AM, Casey Schaufler wrote:
>> On 7/10/2019 6:34 AM, Aaron Goidel wrote:
>>> @@ -3261,6 +3262,26 @@ static int selinux_inode_removexattr(struct dentry
>>> *dentry, const char *name)
>>> return -EACCES;
>>> }
>>>
>>> +static int selinux_inode_notify(struct inode *inode, u64 mask)
>>> +{
>>> + u32 perm = FILE__WATCH; // basic permission, can a watch be set?
>> We don't use // comments in the Linux kernel.
>>
> I thought that we had recently moved into the 21st century on that issue,
> but I don't see it mentioned in coding-style.rst. Maybe we need a Doc update.
Really? Yuck. Next thing you know M4 macros will be allowed.
>
> checkpatch allows C99 comments by default.
> Joe, do you recall about this?
>
> thanks.
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/19 9:38 AM, Casey Schaufler wrote:
> On 7/10/2019 6:34 AM, Aaron Goidel wrote:
>> @@ -3261,6 +3262,26 @@ static int selinux_inode_removexattr(struct dentry
>> *dentry, const char *name)
>> return -EACCES;
>> }
>>
>> +static int selinux_inode_notify(struct inode *inode, u64 mask)
>> +{
>> +u32 perm = FILE__WATCH; // basic permission, can a watch be set?
>
> We don't use // comments in the Linux kernel.
>
I thought that we had recently moved into the 21st century on that issue,
but I don't see it mentioned in coding-style.rst. Maybe we need a Doc update.
checkpatch allows C99 comments by default.
Joe, do you recall about this?
thanks.
--
~Randy
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On 7/10/2019 6:34 AM, Aaron Goidel wrote: > As of now, setting watches on filesystem objects has, at most, applied a > check for read access to the inode, and in the case of fanotify, requires > CAP_SYS_ADMIN. No specific security hook or permission check has been > provided to control the setting of watches. Using any of inotify, dnotify, > or fanotify, it is possible to observe, not only write-like operations, but > even read access to a file. Modeling the watch as being merely a read from > the file is insufficient. That's a very model-specific viewpoint. It is true for a fine-grained model such as SELinux, but not necessarily for a model with more traditional object definitions. I'm not saying you're wrong, I'm saying that stating it as a given assumes your model. You can do that all you want within SELinux, but it doesn't hold when you're talking about the LSM infrastructure. Have you coordinated this with the work that David Howells is doing on generic notifications? > Furthermore, fanotify watches grant more power to > an application in the form of permission events. While notification events > are solely, unidirectional (i.e. they only pass information to the > receiving application), permission events are blocking. Permission events > make a request to the receiving application which will then reply with a > decision as to whether or not that action may be completed. You're not saying why this is an issue. > In order to solve these issues, a new LSM hook is implemented and has been > placed within the system calls for marking filesystem objects with inotify, > fanotify, and dnotify watches. These calls to the hook are placed at the > point at which the target inode has been resolved and are provided with > both the inode and the mask of requested notification events. The mask has > already been translated into common FS_* values shared by the entirety of > the fs notification infrastructure. > > This only provides a hook at the point of setting a watch, and presumes > that permission to set a particular watch implies the ability to receive > all notification about that object which match the mask. This is all that > is required for SELinux. If other security modules require additional hooks > or infrastructure to control delivery of notification, these can be added > by them. It does not make sense for us to propose hooks for which we have > no implementation. The understanding that all notifications received by the > requesting application are all strictly of a type for which the application > has been granted permission shows that this implementation is sufficient in > its coverage. A reasonable approach. It would be *nice* if you had a look at the other security modules to see what they might need from such a hook or hook set. > Fanotify further has the issue that it returns a file descriptor with the > file mode specified during fanotify_init() to the watching process on > event. This is already covered by the LSM security_file_open hook if the > security module implements checking of the requested file mode there. How is this relevant? > The selinux_inode_notify hook implementation works by adding three new > file permissions: watch, watch_reads, and watch_with_perm (descriptions > about which will follow). The hook then decides which subset of these > permissions must be held by the requesting application based on the > contents of the provided mask. The selinux_file_open hook already checks > the requested file mode and therefore ensures that a watching process > cannot escalate its access through fanotify. Thereby increasing the granularity of control available. > The watch permission is the baseline permission for setting a watch on an > object and is a requirement for any watch to be set whatsoever. It should > be noted that having either of the other two permissions (watch_reads and > watch_with_perm) does not imply the watch permission, though this could be > changed if need be. > > The watch_reads permission is required to receive notifications from > read-exclusive events on filesystem objects. These events include accessing > a file for the purpose of reading and closing a file which has been opened > read-only. This distinction has been drawn in order to provide a direct > indication in the policy for this otherwise not obvious capability. Read > access to a file should not necessarily imply the ability to observe read > events on a file. > > Finally, watch_with_perm only applies to fanotify masks since it is the > only way to set a mask which allows for the blocking, permission event. > This permission is needed for any watch which is of this type. Though > fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit > trust to root, which we do not do, and does not support least privilege. > > Signed-off-by: Aaron Goidel > --- > fs/notify/dnotify/dnotify.c | 14 +++--- > fs/notify/fanotify/fanotify_user.c | 11 +-- > fs/notify/i
Re: [RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
On Wed, Jul 10, 2019 at 4:34 PM Aaron Goidel wrote: > > As of now, setting watches on filesystem objects has, at most, applied a > check for read access to the inode, and in the case of fanotify, requires > CAP_SYS_ADMIN. No specific security hook or permission check has been > provided to control the setting of watches. Using any of inotify, dnotify, > or fanotify, it is possible to observe, not only write-like operations, but > even read access to a file. Modeling the watch as being merely a read from > the file is insufficient. Furthermore, fanotify watches grant more power to > an application in the form of permission events. While notification events > are solely, unidirectional (i.e. they only pass information to the > receiving application), permission events are blocking. Permission events > make a request to the receiving application which will then reply with a > decision as to whether or not that action may be completed. > > In order to solve these issues, a new LSM hook is implemented and has been > placed within the system calls for marking filesystem objects with inotify, > fanotify, and dnotify watches. These calls to the hook are placed at the > point at which the target inode has been resolved and are provided with > both the inode and the mask of requested notification events. The mask has > already been translated into common FS_* values shared by the entirety of > the fs notification infrastructure. > > This only provides a hook at the point of setting a watch, and presumes > that permission to set a particular watch implies the ability to receive > all notification about that object which match the mask. This is all that > is required for SELinux. If other security modules require additional hooks > or infrastructure to control delivery of notification, these can be added > by them. It does not make sense for us to propose hooks for which we have > no implementation. The understanding that all notifications received by the > requesting application are all strictly of a type for which the application > has been granted permission shows that this implementation is sufficient in > its coverage. > > Fanotify further has the issue that it returns a file descriptor with the > file mode specified during fanotify_init() to the watching process on > event. This is already covered by the LSM security_file_open hook if the > security module implements checking of the requested file mode there. > > The selinux_inode_notify hook implementation works by adding three new > file permissions: watch, watch_reads, and watch_with_perm (descriptions > about which will follow). The hook then decides which subset of these > permissions must be held by the requesting application based on the > contents of the provided mask. The selinux_file_open hook already checks > the requested file mode and therefore ensures that a watching process > cannot escalate its access through fanotify. > > The watch permission is the baseline permission for setting a watch on an > object and is a requirement for any watch to be set whatsoever. It should > be noted that having either of the other two permissions (watch_reads and > watch_with_perm) does not imply the watch permission, though this could be > changed if need be. > > The watch_reads permission is required to receive notifications from > read-exclusive events on filesystem objects. These events include accessing > a file for the purpose of reading and closing a file which has been opened > read-only. This distinction has been drawn in order to provide a direct > indication in the policy for this otherwise not obvious capability. Read > access to a file should not necessarily imply the ability to observe read > events on a file. > > Finally, watch_with_perm only applies to fanotify masks since it is the > only way to set a mask which allows for the blocking, permission event. > This permission is needed for any watch which is of this type. Though > fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit > trust to root, which we do not do, and does not support least privilege. > > Signed-off-by: Aaron Goidel > --- > fs/notify/dnotify/dnotify.c | 14 +++--- > fs/notify/fanotify/fanotify_user.c | 11 +-- > fs/notify/inotify/inotify_user.c| 12 ++-- > include/linux/lsm_hooks.h | 2 ++ > include/linux/security.h| 7 +++ > security/security.c | 5 + > security/selinux/hooks.c| 22 ++ > security/selinux/include/classmap.h | 2 +- > 8 files changed, 67 insertions(+), 8 deletions(-) > > diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c > index 250369d6901d..e91ce092efb1 100644 > --- a/fs/notify/dnotify/dnotify.c > +++ b/fs/notify/dnotify/dnotify.c > @@ -22,6 +22,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -288,6 +289,16 @@ int fcntl_dirnotify(int fd, struct file
[RFC PATCH] fanotify, inotify, dnotify, security: add security hook for fs notifications
As of now, setting watches on filesystem objects has, at most, applied a check for read access to the inode, and in the case of fanotify, requires CAP_SYS_ADMIN. No specific security hook or permission check has been provided to control the setting of watches. Using any of inotify, dnotify, or fanotify, it is possible to observe, not only write-like operations, but even read access to a file. Modeling the watch as being merely a read from the file is insufficient. Furthermore, fanotify watches grant more power to an application in the form of permission events. While notification events are solely, unidirectional (i.e. they only pass information to the receiving application), permission events are blocking. Permission events make a request to the receiving application which will then reply with a decision as to whether or not that action may be completed. In order to solve these issues, a new LSM hook is implemented and has been placed within the system calls for marking filesystem objects with inotify, fanotify, and dnotify watches. These calls to the hook are placed at the point at which the target inode has been resolved and are provided with both the inode and the mask of requested notification events. The mask has already been translated into common FS_* values shared by the entirety of the fs notification infrastructure. This only provides a hook at the point of setting a watch, and presumes that permission to set a particular watch implies the ability to receive all notification about that object which match the mask. This is all that is required for SELinux. If other security modules require additional hooks or infrastructure to control delivery of notification, these can be added by them. It does not make sense for us to propose hooks for which we have no implementation. The understanding that all notifications received by the requesting application are all strictly of a type for which the application has been granted permission shows that this implementation is sufficient in its coverage. Fanotify further has the issue that it returns a file descriptor with the file mode specified during fanotify_init() to the watching process on event. This is already covered by the LSM security_file_open hook if the security module implements checking of the requested file mode there. The selinux_inode_notify hook implementation works by adding three new file permissions: watch, watch_reads, and watch_with_perm (descriptions about which will follow). The hook then decides which subset of these permissions must be held by the requesting application based on the contents of the provided mask. The selinux_file_open hook already checks the requested file mode and therefore ensures that a watching process cannot escalate its access through fanotify. The watch permission is the baseline permission for setting a watch on an object and is a requirement for any watch to be set whatsoever. It should be noted that having either of the other two permissions (watch_reads and watch_with_perm) does not imply the watch permission, though this could be changed if need be. The watch_reads permission is required to receive notifications from read-exclusive events on filesystem objects. These events include accessing a file for the purpose of reading and closing a file which has been opened read-only. This distinction has been drawn in order to provide a direct indication in the policy for this otherwise not obvious capability. Read access to a file should not necessarily imply the ability to observe read events on a file. Finally, watch_with_perm only applies to fanotify masks since it is the only way to set a mask which allows for the blocking, permission event. This permission is needed for any watch which is of this type. Though fanotify requires CAP_SYS_ADMIN, this is insufficient as it gives implicit trust to root, which we do not do, and does not support least privilege. Signed-off-by: Aaron Goidel --- fs/notify/dnotify/dnotify.c | 14 +++--- fs/notify/fanotify/fanotify_user.c | 11 +-- fs/notify/inotify/inotify_user.c| 12 ++-- include/linux/lsm_hooks.h | 2 ++ include/linux/security.h| 7 +++ security/security.c | 5 + security/selinux/hooks.c| 22 ++ security/selinux/include/classmap.h | 2 +- 8 files changed, 67 insertions(+), 8 deletions(-) diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 250369d6901d..e91ce092efb1 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -288,6 +289,16 @@ int fcntl_dirnotify(int fd, struct file *filp, unsigned long arg) goto out_err; } + /* +* convert the userspace DN_* "arg" to the internal FS_* +* defined in fsnotify +*/ + mask = convert_arg(arg); + +
