Skip certain paths from scanning #2052

jan-cerny · 2023-11-16T16:23:53Z

This PR adds an option to skip selected paths from content evaluation. This will allow users to avoid pollution of the scan results from container guests, various mounts and other filesystems.

Related to: https://issues.redhat.com/browse/RHEL-11925, https://issues.redhat.com/browse/RHEL-4141

Users can configure the paths they want to be skipped by setting the OSCAP_PROBE_IGNORE_PATHS environment variable. If multiple paths should be skipped they need to be separated by a colon.

Review hints:

take a look at the test that is part of this PR
set the OSCAP_PROBE_IGNORE_PATHS and see how it affects the results, compare the results with this variable set and unset, for example:
1. $OSCAP xccdf eval --profile '(all)' --rule xccdf_org.ssgproject.content_rule_selinux_state --report /tmp/report1.html /usr/share/xml/scap/ssg/content/ssg-fedora-ds.xml
2. OSCAP_PROBE_IGNORE_PATHS=/etc/selinux $OSCAP xccdf eval --profile '(all)' --rule xccdf_org.ssgproject.content_rule_selinux_state --report /tmp/report2.html /usr/share/xml/scap/ssg/content/ssg-fedora-ds.xml
3. firefox /tmp/report1.html /tmp/report2.html

evgenyz · 2023-11-17T00:50:31Z

To my taste --ignore-path or --skip-path sound better. I'd maybe even say that OSCAP_PROBE_IGNORE_PATH=/path1/:/path2/path3 is more suitable.

marcusburghardt · 2023-11-17T08:31:28Z

There are already some workarounds for the demand to exclude directories from an oscap scan, as described in this article https://access.redhat.com/solutions/4193501. So there are valid cases when this behavior might be required.

I believe this new option would make it much easier, compared to other workarounds, for users to consciously avoid some directories by whatever reason. Since the idea is to introduce this option without defaults, there is no change of behavior by default while the tool will still offer the option for the user.

jan-cerny · 2023-11-20T13:10:27Z

One concern with this proposal that I have is that we won't be able to "block" the selected paths completely as it's hard and laborous to track all the places across the code where a file path is opened. Also, the results of doing it this way can be different than a solution with wrapping OpenSCAP to a systemd service with paths excluded. This solution prevents probes from collecting items whose path matches the given path. OTOH the systemd service blocks the open calls which return errors. Probes and follow up logic react to these situations differently.

evgenyz · 2023-11-20T13:30:31Z

One concern with this proposal that I have is that we won't be able to "block" the selected paths completely as it's hard and laborous to track all the places across the code where a file path is opened. Also, the results of doing it this way can be different than a solution with wrapping OpenSCAP to a systemd service with paths excluded. This solution prevents probes from collecting items whose path matches the given path. OTOH the systemd service blocks the open calls which return errors. Probes and follow up logic react to these situations differently.

That's exactly why we haven't tried to actually implement this feature in the past (it has been discussed multiple times). On the other hand, since this feature is not standardized in any way, we could do something and then improve based on the feedback. We should just have to make sure it is clear that the feature is experimental and technically violates OVAL.

The systemd approach is more consistent, but less convenient. I can suggest exploring the option of doing what systemd is doing, but on the scanner side (which might not be easy as well).

evgenyz · 2023-11-20T13:47:24Z

The systemd approach is more consistent, but less convenient. I can suggest exploring the option of doing what systemd is doing, but on the scanner side (which might not be easy as well).

But, to be noted, the issue with users is not connected to the fact that systemd service is not convenient, but rather to the idea that skipping some paths is not acceptable, as far as I understand. This, if true, makes the whole effort a bit questionable.

marcusburghardt · 2023-11-23T09:21:17Z

The systemd approach is more consistent, but less convenient. I can suggest exploring the option of doing what systemd is doing, but on the scanner side (which might not be easy as well).

But, to be noted, the issue with users is not connected to the fact that systemd service is not convenient, but rather to the idea that skipping some paths is not acceptable, as far as I understand. This, if true, makes the whole effort a bit questionable.

We should not expect this option will be used in all cases. The users are free to decide if they want or not to use this new option for their cases. Users who would benefit from this option currently have to use an inconvenient workaround. This option will make their experiences better.

Based on the @jan-cerny concerns, I think makes a lot of sense to communicate this option as experimental. It doesn't need to be perfect now. We can improve on demand.

jan-cerny · 2023-11-29T16:32:08Z

I have slightly updated this draft. Now, the paths can be specified using the environment variable OSCAP_PROBE_IGNORE_PATHS and need to be separated by a colon. I have removed the command line option that was originally here.

I also started a test in the test suite.

This cast and rename of the `void *arg` to `struct pfdata *pfd` isn't necessary, we can change the argument type to the actual type, because the function is called only once and therefore always with an argument of the `struct pfdata *` type.

People will be able to tell probes to skip certain paths during content evaluation. They can specify a list of paths by setting the `OSCAP_PROBE_IGNORE_PATHS` environment variable. The paths in this list should be separated by a colon.

This cast and rename of the `void *arg` to `struct pfdata *pfd` isn't necessary, we can change the argument type to the actual type, because the function is called only once and therefore always with an argument of the `struct pfdata *` type.

Extend test_skip_paths to test the "skip selected paths" feature also in the filehash58 probe.

Extend test_skip_paths to test the "skip selected paths" feature also in the xmlfilecontent probe.

jan-cerny · 2023-12-18T17:12:39Z

I have add the feature to other relevant probes. I improved the test. Also, I have rebased this PR on the top of the latest upstream maint-1.3 branch.

jan-cerny · 2023-12-21T14:06:07Z

The CI TF fail on RH will be fixed by #2067.

evgenyz

There are some quick nitpicks. I'll take a bit more time to test it thoroughly.

Question right from the top of my head, though: does it work in offline mode?

docs/manual/manual.adoc

tests/API/OVAL/skip_paths/test.xml

tests/API/OVAL/skip_paths/CMakeLists.txt

src/OVAL/probes/probe-api.c

jan-cerny · 2024-01-10T11:55:55Z

Yes, it should work offline. I have add an offline version of the test. Then I have add a missing EOF, updated the documentation about the paths and I have moved the path_startswith function to utils module.

evgenyz

So, in addition to the oscap_path_startswith function behavior, I have a couple more concerns regarding the whole feature:

Windows support. We don't have to provide it, but silently doing a bad thing is not very friendly to our users. We can either implement it correctly or ignore the environment variable with a warning that it is being ignored because the feature is not implemented for Windows platform.
Silent magic. Imagine receiving a log with an ARF file and a question like "why all my files in /etc are ignored". User will obviously forget to mention the environment variable.
I will not insist on adding anything to this PR, but I'll be very hesitant to release this feature (and Introduce a limit of collected items #2051) without an implementation of Dump all env. variables that affects the behaviour on a DEBUG or INFO log level #2063.

docs/manual/manual.adoc

src/common/util.c

jan-cerny · 2024-01-16T14:13:44Z

I have add a simple unit test, updated documentation and added a Windows warning.

evgenyz · 2024-01-16T15:02:56Z

@jan-cerny Do you have any ideas about probe_behavior/collect_limit.sh failure in centos-8?

jan-cerny · 2024-01-16T15:05:09Z

/packit retest-failed

evgenyz

LGTM. Thank you!

jan-cerny mentioned this pull request Nov 20, 2023

Introduce a limit of collected items #2051

Merged

marcusburghardt added the enhancement label Nov 23, 2023

jan-cerny force-pushed the block_paths branch from fa8af58 to 7c344f7 Compare November 29, 2023 16:27

marcusburghardt mentioned this pull request Nov 30, 2023

Improve Performance on rules probing the whole file system ComplianceAsCode/content#11319

Merged

10 tasks

jan-cerny added this to the 1.3.10 milestone Dec 18, 2023

jan-cerny added 11 commits December 18, 2023 17:08

Avoid a cast

b62069e

This cast and rename of the `void *arg` to `struct pfdata *pfd` isn't necessary, we can change the argument type to the actual type, because the function is called only once and therefore always with an argument of the `struct pfdata *` type.

Add ability to block paths

1e4b8ae

People will be able to tell probes to skip certain paths during content evaluation. They can specify a list of paths by setting the `OSCAP_PROBE_IGNORE_PATHS` environment variable. The paths in this list should be separated by a colon.

Add a simple test for the skip paths feature

63d2a79

Use blocked paths in filehash58 probe

2b367b6

Use blocked paths in filehash probe

aa9ffd0

Use blocked paths in textfilecontent probe

a2201c1

Avoid a cast

551f972

This cast and rename of the `void *arg` to `struct pfdata *pfd` isn't necessary, we can change the argument type to the actual type, because the function is called only once and therefore always with an argument of the `struct pfdata *` type.

Use blocked paths in xmlfilecontent probe

e4ea549

Use blocked paths in yamlfilecontent probe

830ea1d

Use blocked paths in fileextendedattribute probe

65a40c5

Use blocked paths in symlink probe

1604943

jan-cerny force-pushed the block_paths branch from a0547b5 to 1604943 Compare December 18, 2023 16:11

jan-cerny added 3 commits December 18, 2023 17:27

Add sensible comments

129a118

Extend test_skip_paths

a033aef

Extend test_skip_paths to test the "skip selected paths" feature also in the filehash58 probe.

Extend test_skip_paths

c316bc4

Extend test_skip_paths to test the "skip selected paths" feature also in the xmlfilecontent probe.

jan-cerny requested a review from marcusburghardt December 18, 2023 17:12

jan-cerny marked this pull request as ready for review January 5, 2024 08:04

evgenyz self-requested a review January 9, 2024 23:16

evgenyz requested changes Jan 9, 2024

View reviewed changes

docs/manual/manual.adoc Outdated Show resolved Hide resolved

tests/API/OVAL/skip_paths/test.xml Outdated Show resolved Hide resolved

tests/API/OVAL/skip_paths/CMakeLists.txt Show resolved Hide resolved

evgenyz reviewed Jan 10, 2024

View reviewed changes

src/OVAL/probes/probe-api.c Outdated Show resolved Hide resolved

jan-cerny added 4 commits January 10, 2024 10:03

Add an offline version of the test

592dd6a

Add a missing newline at EOF

8cb70c5

Move a function to utils

b50e1ba

Specify that the paths are absolute

faa742a

evgenyz requested changes Jan 11, 2024

View reviewed changes

docs/manual/manual.adoc Outdated Show resolved Hide resolved

src/common/util.c Show resolved Hide resolved

jan-cerny added 3 commits January 16, 2024 14:52

Add a simple unit test for oscap_path_startswith

e9e8558

Add a canonical path to the documentation

58e409c

Add a warning on Windows

2d20f33

evgenyz approved these changes Jan 16, 2024

View reviewed changes

evgenyz merged commit 9b38fc2 into OpenSCAP:maint-1.3 Jan 16, 2024
19 checks passed

evgenyz mentioned this pull request Feb 1, 2024

HTML Report Creation Fails With XPath error: growing nodeset hit limit #2082

Closed

evgenyz mentioned this pull request Apr 2, 2024

oscap #2099

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip certain paths from scanning #2052

Skip certain paths from scanning #2052

jan-cerny commented Nov 16, 2023 •

edited

Loading

evgenyz commented Nov 17, 2023

marcusburghardt commented Nov 17, 2023

jan-cerny commented Nov 20, 2023

evgenyz commented Nov 20, 2023

evgenyz commented Nov 20, 2023

marcusburghardt commented Nov 23, 2023

jan-cerny commented Nov 29, 2023

jan-cerny commented Dec 18, 2023

jan-cerny commented Dec 21, 2023

evgenyz left a comment

jan-cerny commented Jan 10, 2024

evgenyz left a comment •

edited

Loading

jan-cerny commented Jan 16, 2024

evgenyz commented Jan 16, 2024 •

edited

Loading

jan-cerny commented Jan 16, 2024

evgenyz left a comment

Skip certain paths from scanning #2052

Skip certain paths from scanning #2052

Conversation

jan-cerny commented Nov 16, 2023 • edited Loading

evgenyz commented Nov 17, 2023

marcusburghardt commented Nov 17, 2023

jan-cerny commented Nov 20, 2023

evgenyz commented Nov 20, 2023

evgenyz commented Nov 20, 2023

marcusburghardt commented Nov 23, 2023

jan-cerny commented Nov 29, 2023

jan-cerny commented Dec 18, 2023

jan-cerny commented Dec 21, 2023

evgenyz left a comment

Choose a reason for hiding this comment

jan-cerny commented Jan 10, 2024

evgenyz left a comment • edited Loading

Choose a reason for hiding this comment

jan-cerny commented Jan 16, 2024

evgenyz commented Jan 16, 2024 • edited Loading

jan-cerny commented Jan 16, 2024

evgenyz left a comment

Choose a reason for hiding this comment

jan-cerny commented Nov 16, 2023 •

edited

Loading

evgenyz left a comment •

edited

Loading

evgenyz commented Jan 16, 2024 •

edited

Loading