RFE: add fsnotify test #115

rprobaina · 2025-05-30T21:04:52Z

Simple test case to verify that records related to fsnotify events are captured as expected.

This test case adds full coverage to the following audit_fsnotify.c functions, which have not been exercised by any other test case of the test suite:

audit_mark_handle_event()
audit_autoremove_mark_rule()
audit_mark_log_rule_change()

Signed-off-by: Ricardo Robaina rrobaina@redhat.com

pcmoore · 2025-06-24T19:01:47Z

tests/fsnotify/test

+
+# set the file watch
+my $key = key_gen();
+system("auditctl -a always,exit -F exe=$filename -S all -k $key");


Shouldn't this be auditctl -a always,exit -F path=$filename -S all -k $key?

Hi @pcmoore ,

Thanks for reviewing this PR!

I've done a coverage analysis using GCOV, and I'm using the data I got to guide me to test uncovered parts of the audit subsystem. I've chosen to use exe in this case to exercise the functions audit_mark_handle_event, audit_autoremove_mark_rule, and audit_mark_log_rule_change, which are not covered by any other test case in the suite. Using path does not exercise these functions.

exe - audit_mark_handle_event 100.00 audit_autoremove_mark_rule 100.00 audit_mark_log_rule_change 100.00 path - audit_mark_handle_event 0.00 audit_autoremove_mark_rule 0.00 audit_mark_log_rule_change 0.00

I hope that makes sense.

I think it would be better to filter based on the pathname here (path=) since this is just a normal file, and not an executable. The executable filter (exe=) is really intended as an easier-to-use, and more durable, variant of the PID filter.

As far as test code coverage is concerned, I agree that improving the coverage of this test suite is an excellent goal, and I applaud your efforts here - thank you! However, I believe the "exec_name" tests have a -F exe= filter which should trigger those functions, although it has been a while since I have looked at the related kernel code. Is there a reason why the exec_name tests do not exercise those kernel functions while your proposed filter here does?

I agree, Paul. It makes more sense to use path in this test, indeed. In fact, I double-checked my findings and noticed that using path also covers those 3 functions' code paths. For some odd reason, GCOV does not get that, tough.

I have just updated this PR, addressing all your points. Please let me know if there's anything else to adjust.

Thanks for letting me know you appreciate this work I'm doing to improve the test coverage. It means a lot to me!

pcmoore · 2025-06-24T19:02:50Z

tests/fsnotify/test

+    }
+}
+ok($found_add_rule);
+ok($found_autoremove_rule);


Should we also check for a record that $filename has been deleted/unlinked?

I think so, @pcmoore . Thanks for bringing it to my attention. I missed that.

However, when I started working on the V2 I noticed that the PATH DELETE and SYSCALL unlinkat events are not generated when using `exe. This behavior is not expected, right? I think we found a bug.

exe ---- type=PROCTITLE msg=audit(07/08/2025 09:48:03.913:118274) : proctitle=auditctl -a always,exit -F exe /tmp/audit-testsuite-g1DO/file-ytrT -S all -k testsuite-1751978883-UIBkgPdI type=SYSCALL msg=audit(07/08/2025 09:48:03.913:118274) : arch=x86_64 syscall=sendto success=yes exit=1120 a0=0x4 a1=0x7ffdc948e2d0 a2=0x460 a3=0x0 items=0 ppid=264149 pid=264154 auid=rrobaina uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=pts7 ses=3 comm=auditctl exe=/usr/sbin/auditctl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) type=CONFIG_CHANGE msg=audit(07/08/2025 09:48:03.913:118274) : auid=rrobaina ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=add_rule key=testsuite-1751978883-UIBkgPdI list=exit res=yes path ---- type=PROCTITLE msg=audit(07/08/2025 09:47:22.986:118264) : proctitle=auditctl -a always,exit -F path /tmp/audit-testsuite-ZpyF/file-oxxi -S all -k testsuite-1751978842-PrIgixtn type=SYSCALL msg=audit(07/08/2025 09:47:22.986:118264) : arch=x86_64 syscall=sendto success=yes exit=1120 a0=0x4 a1=0x7ffda97bffd0 a2=0x460 a3=0x0 items=0 ppid=264106 pid=264111 auid=rrobaina uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=pts7 ses=3 comm=auditctl exe=/usr/sbin/auditctl subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=(null) type=CONFIG_CHANGE msg=audit(07/08/2025 09:47:22.986:118264) : auid=rrobaina ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=add_rule key=testsuite-1751978842-PrIgixtn list=exit res=yes ---- type=PROCTITLE msg=audit(07/08/2025 09:47:22.986:118265) : proctitle=rm -Rf /tmp/audit-testsuite-ZpyF type=PATH msg=audit(07/08/2025 09:47:22.986:118265) : item=1 name=file-oxxi inode=4199 dev=00:2b mode=file,600 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=DELETE cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0 type=PATH msg=audit(07/08/2025 09:47:22.986:118265) : item=0 name=/home/rrobaina/src/audit/audit-testsuite/tests/fsnotify inode=4196 dev=00:2b mode=dir,700 ouid=root ogid=root rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=PARENT cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0 type=CWD msg=audit(07/08/2025 09:47:22.986:118265) : cwd=/home/rrobaina/src/audit/audit-testsuite/tests/fsnotify type=SYSCALL msg=audit(07/08/2025 09:47:22.986:118265) : arch=x86_64 syscall=unlinkat success=yes exit=0 a0=0x4 a1=0x564a99256c10 a2=0x0 a3=0x1a items=2 ppid=264106 pid=264113 auid=rrobaina uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=pts7 ses=3 comm=rm exe=/usr/bin/rm subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=testsuite-1751978842-PrIgixtn

I'm looking forward to hearing back from you. If it's indeed a bug, I'll work on a fix.

Thanks again for your review.
-Ricardo

As documented in the auditctl(8) manpage, the executable filter only applies to a process that is executing:

Absolute path to application that while executing this rule will apply to. It supports = and != operators. Note that you can only use this once for each rule.

... with that in mind, it doesn't appear that this is a bug, but rather expected behavior.

Thanks for clarifying it, Paul. I've added test cases to check if the file was successfully deleted/unlinked.

pcmoore · 2025-08-29T11:21:49Z

tests/fsnotify/test

+
+# set the file watch
+my $key = key_gen();
+system("auditctl -a always,exit -F exe=$filename -S all -k $key");


I think it would be better to filter based on the pathname here (path=) since this is just a normal file, and not an executable. The executable filter (exe=) is really intended as an easier-to-use, and more durable, variant of the PID filter.

As far as test code coverage is concerned, I agree that improving the coverage of this test suite is an excellent goal, and I applaud your efforts here - thank you! However, I believe the "exec_name" tests have a -F exe= filter which should trigger those functions, although it has been a while since I have looked at the related kernel code. Is there a reason why the exec_name tests do not exercise those kernel functions while your proposed filter here does?

pcmoore · 2025-08-29T11:24:36Z

tests/fsnotify/test

+    }
+}
+ok($found_add_rule);
+ok($found_autoremove_rule);


As documented in the auditctl(8) manpage, the executable filter only applies to a process that is executing:

Absolute path to application that while executing this rule will apply to. It supports = and != operators. Note that you can only use this once for each rule.

... with that in mind, it doesn't appear that this is a bug, but rather expected behavior.

Simple test case to verify that records related to fsnotify events are captured as expected. This test case adds full coverage to the following audit_fsnotify.c functions, which have not been exercised by any other test case of the test suite: - audit_mark_handle_event() - audit_autoremove_mark_rule() - audit_mark_log_rule_change() Signed-off-by: Ricardo Robaina <rrobaina@redhat.com>

rprobaina mentioned this pull request May 30, 2025

CI jobs fail due to deprecated ubuntu-20.04 image #116

Closed

pcmoore changed the title ~~tests/fsnotify: add fsnotify test~~ RFE: add fsnotify test Jun 24, 2025

pcmoore added the enhancement label Jun 24, 2025

pcmoore reviewed Jun 24, 2025

View reviewed changes

pcmoore added pending/info pending/revision labels Jun 24, 2025

rprobaina requested a review from pcmoore July 8, 2025 13:15

pcmoore requested changes Aug 29, 2025

View reviewed changes

rprobaina force-pushed the fsnotify branch from 0c761f4 to de03c30 Compare October 15, 2025 11:11

rprobaina requested a review from pcmoore October 15, 2025 11:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

RFE: add fsnotify test #115

RFE: add fsnotify test #115

Uh oh!

rprobaina commented May 30, 2025

Uh oh!

pcmoore Jun 24, 2025

Uh oh!

rprobaina Jul 8, 2025

Uh oh!

pcmoore Aug 29, 2025

Uh oh!

rprobaina Oct 15, 2025

Uh oh!

pcmoore Jun 24, 2025

Uh oh!

rprobaina Jul 8, 2025 •

edited

Loading

Uh oh!

pcmoore Aug 29, 2025

Uh oh!

rprobaina Oct 15, 2025

Uh oh!

pcmoore Aug 29, 2025

Uh oh!

pcmoore Aug 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

RFE: add fsnotify test #115

Are you sure you want to change the base?

RFE: add fsnotify test #115

Uh oh!

Conversation

rprobaina commented May 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rprobaina Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rprobaina Jul 8, 2025 •

edited

Loading