| Message ID | cover.1541639254.git.mbobrowski@mbobrowski.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | fanotify: introduce new event mask FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM | expand |
On Thu 08-11-18 14:04:10, Matthew Bobrowski wrote: > Currently, the fanotify API does not provide a means for user space > applications to receive events when a file has been opened specifically > for execution. New event masks FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM have > been introduced in order to provide users with this capability. > > These event types, when either are explicitly requested by the user, will > be returned within the event mask when a marked file object is being > opened has __FMODE_EXEC set as one of the flags for open_flag. > > Linux is used as an operating system in some products, with an environment > that can be certified under the Common Criteria Operating System > Protection Profile (OSPP). This is a formal threat model for a class of > technology. It requires specific countermeasures to mitigate threats. It > requires documentation to explain how a product implements these > countermeasures. It requires proof via a test suite to demonstrate that > the requirements are met, observed and checked by an independent qualified > third party. The latest set of requirements for OSPP v4.2 can be found > here: > > https://www.niap-ccevs.org/Profile/Info.cfm?PPID=424&id=424 > > If you look on page 58, you will see the following requirement: > > FPT_SRP_EXT.1 Software Restriction Policies FPT_SRP_EXT.1.1 > administrator specified [selection: > file path, > file digital signature, > version, > hash, > [assignment: other characteristics] > ] > > This patch is to help aid in meeting this requirement. > > Changes since v6: > * Reordered patches within the patch series for the sake of > correctness. > * Updated patch that contains FAN_OPEN_EXEC_PERM functionality to > include separate call to fsnotify_parent()/fsnotify() which is > used to mitigate merging of permission events. > > Note that this set of patches are based on Amir's fsnotify-fixes branch, > which has already been posted through for review. For those interested, > this branch can be found here: > https://github.com/amir73il/linux/commits/fsnotify-fixes > > LTP tests for this feature are on my 'fanotify-exec' branch here: > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > that contains the test cases are provided below: > > syscalls/fanotify03: test cases have been updated to cover > FAN_OPEN_EXEC_PERM events > syscalls/fanotify12: newly introduced LTP test file to cover > FAN_OPEN_EXEC events I have been wondering for a while why the testcases passed when ignore mask hasn't been properly treated in fanotify_group_event_mask() but then I realized that the generic code will not even call to fanotify if ignore masks result in clearing the event. Anyway, the rest of the series looks OK (I've updated the changelog of 4/4 as requested by Amir) and tests pass fine so once I get confirmation from you regarding 1/4, I'll push everything out. Honza
> > LTP tests for this feature are on my 'fanotify-exec' branch here: > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > that contains the test cases are provided below: > > > > syscalls/fanotify03: test cases have been updated to cover > > FAN_OPEN_EXEC_PERM events > > syscalls/fanotify12: newly introduced LTP test file to cover > > FAN_OPEN_EXEC events > > I have been wondering for a while why the testcases passed when ignore mask > hasn't been properly treated in fanotify_group_event_mask() but then I > realized that the generic code will not even call to fanotify if ignore > masks result in clearing the event. So does that means we have missing test coverage? I think the idea of this patch was that FAN_MARK_INODE, FAN_OPEN | FAN_OPEN_EXEC + FAN_MARK_MOUNT, FAN_MARK_IGNORED_MASK | FAN_OPEN_EXEC Will result with event with mask FAN_OPEN without FAN_OPEN_EXEC in spite the implementation of patch 2/4 using mask |= FS_OPEN_EXEC. No? Thanks, Amir.
On Tue, Nov 13, 2018 at 8:01 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > LTP tests for this feature are on my 'fanotify-exec' branch here: > > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > > that contains the test cases are provided below: > > > > > > syscalls/fanotify03: test cases have been updated to cover > > > FAN_OPEN_EXEC_PERM events > > > syscalls/fanotify12: newly introduced LTP test file to cover > > > FAN_OPEN_EXEC events > > > > I have been wondering for a while why the testcases passed when ignore mask > > hasn't been properly treated in fanotify_group_event_mask() but then I > > realized that the generic code will not even call to fanotify if ignore > > masks result in clearing the event. > > So does that means we have missing test coverage? > This is covered by test case #3 of Matthew's proposed LTP test. https://github.com/matthewbobrowski/ltp/commit/9e350fe15a5423d896ed0e8e147edc15bee13b42#diff-2bb8ddff24b3a031be0f64354262e587R76 > I think the idea of this patch was that > FAN_MARK_INODE, FAN_OPEN | FAN_OPEN_EXEC > + > FAN_MARK_MOUNT, FAN_MARK_IGNORED_MASK | FAN_OPEN_EXEC > Not even mount mark. ignore mask on the same inode mark as well. Thanks, Amir.
On Wed 14-11-18 05:43:25, Amir Goldstein wrote: > On Tue, Nov 13, 2018 at 8:01 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > LTP tests for this feature are on my 'fanotify-exec' branch here: > > > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > > > that contains the test cases are provided below: > > > > > > > > syscalls/fanotify03: test cases have been updated to cover > > > > FAN_OPEN_EXEC_PERM events > > > > syscalls/fanotify12: newly introduced LTP test file to cover > > > > FAN_OPEN_EXEC events > > > > > > I have been wondering for a while why the testcases passed when ignore mask > > > hasn't been properly treated in fanotify_group_event_mask() but then I > > > realized that the generic code will not even call to fanotify if ignore > > > masks result in clearing the event. > > > > So does that means we have missing test coverage? > > > > This is covered by test case #3 of Matthew's proposed LTP test. > https://github.com/matthewbobrowski/ltp/commit/9e350fe15a5423d896ed0e8e147edc15bee13b42#diff-2bb8ddff24b3a031be0f64354262e587R76 This testcase does not catch the bug we had in fanotify_group_event_mask() because the masking by mark->mask already hides the fact that we failed to apply the ignore mask. What does catch this kind of bug (tested) is a testcase (admittedly somewhat silly) like this: { "inode mark, FAN_OPEN | FAN_OPEN_EXEC events, ignore FAN_OPEN_EXEC", INIT_FANOTIFY_MARK_TYPE(INODE), FAN_OPEN | FAN_OPEN_EXEC, FAN_OPEN_EXEC, 2, {FAN_OPEN, FAN_OPEN} }, A real variant of this would be FAN_OPEN | FAN_OPEN_EXEC on mount, ignore FAN_OPEN on inode. Then we should just get one FAN_OPEN_EXEC but with the bug we'd get FAN_OPEN | FAN_OPEN_EXEC. But creating such test would be slightly more involved. But probably it is worth it. Matthew? Also I have noticed that fanotify12 test has a bug that it reports: fanotify12.c:220: FAIL: Received event: mask=1020, pid=5142 (expected 5142), fd=5 i.e., it reports expected pid instead of expected mask when mask does not match. Can you please fix it Matthew? Honza
On Wed, Nov 14, 2018 at 2:02 PM Jan Kara <jack@suse.cz> wrote: > > On Wed 14-11-18 05:43:25, Amir Goldstein wrote: > > On Tue, Nov 13, 2018 at 8:01 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > LTP tests for this feature are on my 'fanotify-exec' branch here: > > > > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > > > > that contains the test cases are provided below: > > > > > > > > > > syscalls/fanotify03: test cases have been updated to cover > > > > > FAN_OPEN_EXEC_PERM events > > > > > syscalls/fanotify12: newly introduced LTP test file to cover > > > > > FAN_OPEN_EXEC events > > > > > > > > I have been wondering for a while why the testcases passed when ignore mask > > > > hasn't been properly treated in fanotify_group_event_mask() but then I > > > > realized that the generic code will not even call to fanotify if ignore > > > > masks result in clearing the event. > > > > > > So does that means we have missing test coverage? > > > > > > > This is covered by test case #3 of Matthew's proposed LTP test. > > https://github.com/matthewbobrowski/ltp/commit/9e350fe15a5423d896ed0e8e147edc15bee13b42#diff-2bb8ddff24b3a031be0f64354262e587R76 > > This testcase does not catch the bug we had in fanotify_group_event_mask() > because the masking by mark->mask already hides the fact that we failed to > apply the ignore mask. > > What does catch this kind of bug (tested) is a testcase (admittedly > somewhat silly) like this: > > { > "inode mark, FAN_OPEN | FAN_OPEN_EXEC events, ignore FAN_OPEN_EXEC", > INIT_FANOTIFY_MARK_TYPE(INODE), > FAN_OPEN | FAN_OPEN_EXEC, > FAN_OPEN_EXEC, > 2, > {FAN_OPEN, FAN_OPEN} > }, > Yah. that is simple to add. Matthew please add it to your test. > A real variant of this would be FAN_OPEN | FAN_OPEN_EXEC on mount, ignore > FAN_OPEN on inode. Then we should just get one FAN_OPEN_EXEC but with the bug > we'd get FAN_OPEN | FAN_OPEN_EXEC. > > But creating such test would be slightly more involved. But probably it is > worth it. Matthew? Not a problem to add a test case to fanotify10 which checks combination of different mark type with ignore mask. Matthew, you should have no problem to extend fanotify10 by making the mask and ignore mask and result mask variables of the test case. Please make that change based on fanotify_sb branch, which extends fanotify10 to filesystem mark test cases. Thanks, Amir.
On Wed, Nov 14, 2018 at 01:02:47PM +0100, Jan Kara wrote: > > > > > LTP tests for this feature are on my 'fanotify-exec' branch here: > > > > > https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files > > > > > that contains the test cases are provided below: > > > > > > > > > > syscalls/fanotify03: test cases have been updated to cover > > > > > FAN_OPEN_EXEC_PERM events > > > > > syscalls/fanotify12: newly introduced LTP test file to cover > > > > > FAN_OPEN_EXEC events > > > > > > > > I have been wondering for a while why the testcases passed when ignore mask > > > > hasn't been properly treated in fanotify_group_event_mask() but then I > > > > realized that the generic code will not even call to fanotify if ignore > > > > masks result in clearing the event. > > > > > > So does that means we have missing test coverage? > > > > > > > This is covered by test case #3 of Matthew's proposed LTP test. > > https://github.com/matthewbobrowski/ltp/commit/9e350fe15a5423d896ed0e8e147edc15bee13b42#diff-2bb8ddff24b3a031be0f64354262e587R76 > > This testcase does not catch the bug we had in fanotify_group_event_mask() > because the masking by mark->mask already hides the fact that we failed to > apply the ignore mask. > > What does catch this kind of bug (tested) is a testcase (admittedly > somewhat silly) like this: > > { > "inode mark, FAN_OPEN | FAN_OPEN_EXEC events, ignore FAN_OPEN_EXEC", > INIT_FANOTIFY_MARK_TYPE(INODE), > FAN_OPEN | FAN_OPEN_EXEC, > FAN_OPEN_EXEC, > 2, > {FAN_OPEN, FAN_OPEN} > }, I've incorporated this^ test as part of my test cases. All tests, this one included, are passing on kernel built on your 'fsnotify' branch. You can find the updated test case file here: https://github.com/matthewbobrowski/ltp/commit/d1d57d5bda8db49a26624c7737c2db88ea90f9db > A real variant of this would be FAN_OPEN | FAN_OPEN_EXEC on mount, ignore > FAN_OPEN on inode. Then we should just get one FAN_OPEN_EXEC but with the bug > we'd get FAN_OPEN | FAN_OPEN_EXEC. > > But creating such test would be slightly more involved. But probably it is > worth it. Matthew? Yeah, this shouldn't be too difficult to add at all, but as Amir pointed out, I'd probably be in favour of putting this into a different test case i.e. one which deals with mounts/filesystem mark types. > Also I have noticed that fanotify12 test has a bug that it reports: > > fanotify12.c:220: FAIL: Received event: mask=1020, pid=5142 (expected 5142), fd=5 > > i.e., it reports expected pid instead of expected mask when mask does not > match. Can you please fix it Matthew? Sure, a fix for this has also been applied here: https://github.com/matthewbobrowski/ltp/commit/d1d57d5bda8db49a26624c7737c2db88ea90f9db
Currently, the fanotify API does not provide a means for user space applications to receive events when a file has been opened specifically for execution. New event masks FAN_OPEN_EXEC and FAN_OPEN_EXEC_PERM have been introduced in order to provide users with this capability. These event types, when either are explicitly requested by the user, will be returned within the event mask when a marked file object is being opened has __FMODE_EXEC set as one of the flags for open_flag. Linux is used as an operating system in some products, with an environment that can be certified under the Common Criteria Operating System Protection Profile (OSPP). This is a formal threat model for a class of technology. It requires specific countermeasures to mitigate threats. It requires documentation to explain how a product implements these countermeasures. It requires proof via a test suite to demonstrate that the requirements are met, observed and checked by an independent qualified third party. The latest set of requirements for OSPP v4.2 can be found here: https://www.niap-ccevs.org/Profile/Info.cfm?PPID=424&id=424 If you look on page 58, you will see the following requirement: FPT_SRP_EXT.1 Software Restriction Policies FPT_SRP_EXT.1.1 administrator specified [selection: file path, file digital signature, version, hash, [assignment: other characteristics] ] This patch is to help aid in meeting this requirement. Changes since v6: * Reordered patches within the patch series for the sake of correctness. * Updated patch that contains FAN_OPEN_EXEC_PERM functionality to include separate call to fsnotify_parent()/fsnotify() which is used to mitigate merging of permission events. Note that this set of patches are based on Amir's fsnotify-fixes branch, which has already been posted through for review. For those interested, this branch can be found here: https://github.com/amir73il/linux/commits/fsnotify-fixes LTP tests for this feature are on my 'fanotify-exec' branch here: https://github.com/matthewbobrowski/ltp/commits/fanotify_exec. The files that contains the test cases are provided below: syscalls/fanotify03: test cases have been updated to cover FAN_OPEN_EXEC_PERM events syscalls/fanotify12: newly introduced LTP test file to cover FAN_OPEN_EXEC events All LTP tests have been run against this series of patches and all are returning as a pass. The man pages updates for these newly defined event masks can be found here: https://github.com/matthewbobrowski/man-pages/commits/fanotify_exec I would like to thank both Amir and Jan for their time and help, highly appreciated once again. Matthew Bobrowski (4): fanotify: return only user requested event types in event mask fanotify: introduce new event mask FAN_OPEN_EXEC fsnotify: refactor fsnotify_parent()/fsnotify() paired calls when event is on path fanotify: introduce new event mask FAN_OPEN_EXEC_PERM fs/notify/fanotify/fanotify.c | 29 ++++++++------- fs/notify/fsnotify.c | 2 +- include/linux/fanotify.h | 5 +-- include/linux/fsnotify.h | 61 +++++++++++++++++++------------- include/linux/fsnotify_backend.h | 11 ++++-- include/uapi/linux/fanotify.h | 2 ++ 6 files changed, 67 insertions(+), 43 deletions(-)