Review rpm_verify_ownership rule #11333
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Also update warning about high consume of system resources in some scenarios.
The OVAL check was inneficient by searching the RPM database twice and consequently creating two similar objects and two tests. The logic was simplified to one single test, one single query in the RPM databases and one single object covering all necessary cases. Besides the simplification, the performance during the check was also improved.
marcusburghardt
added
refactoring
|
Start a new ephemeral environment with changes proposed in this pull request: rhel8 (from CTF) Environment (using Fedora as testing environment) Fedora Testing Environment Oracle Linux 8 Environment |
|
This datastream diff is auto generated by the check Click here to see the full diffNew content has different text for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_ownership'.
--- xccdf_org.ssgproject.content_rule_rpm_verify_ownership
+++ xccdf_org.ssgproject.content_rule_rpm_verify_ownership
@@ -3,22 +3,27 @@
Verify and Correct Ownership with RPM
[description]:
-The RPM package management system can check file ownership
-permissions of installed software packages, including many that are
-important to system security. After locating a file with incorrect
-permissions, which can be found with
+The RPM package management system can check file ownership permissions of installed software
+packages, including many that are important to system security. After locating a file with
+incorrect permissions, which can be found with:
rpm -Va | awk '{ if (substr($0,6,1)=="U" || substr($0,7,1)=="G") print $NF }'
run the following command to determine which package owns it:
$ rpm -qf FILENAME
-Next, run the following command to reset its permissions to
-the correct values:
+Next, run the following command to reset its permissions to the correct values:
$ sudo rpm --setugids PACKAGENAME
[warning]:
Profiles may require that specific files be owned by root while the default owner defined
-by the vendor is different.
-Such files will be reported as a finding and need to be evaluated according to your policy
-and deployment environment.
+by the vendor is different. Such files will be reported as a finding and need to be
+evaluated according to your policy and deployment environment.
+
+[warning]:
+This rule can take a long time to perform the check and might consume a considerable
+amount of resources depending on the number of packages present on the system. It is not a
+problem in most cases, but especially systems with a large number of installed packages
+can be affected.
+
+See https://access.redhat.com/articles/6999111.
[reference]:
1
@@ -354,10 +359,9 @@
6.1.9
[rationale]:
-Ownership of binaries and configuration files that is incorrect
-could allow an unauthorized user to gain privileges that they should
-not have. The ownership set by the vendor should be maintained. Any
-deviations from this baseline should be investigated.
+Ownership of binaries and configuration files that is incorrect could allow an unauthorized
+user to gain privileges that they should not have. The ownership set by the vendor should be
+maintained. Any deviations from this baseline should be investigated.
[ident]:
CCE-82196-7
OVAL for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_ownership' differs.
--- oval:ssg-rpm_verify_ownership:def:1
+++ oval:ssg-rpm_verify_ownership:def:1
@@ -1,3 +1,2 @@
criteria AND
-criterion oval:ssg-test_verify_all_rpms_user_ownership:tst:1
-criterion oval:ssg-test_verify_all_rpms_group_ownership:tst:1
+criterion oval:ssg-test_rpm_verify_ownership_verify_all_rpms_ownership:tst:1 |
10 tasks
|
/packit retest-failed |
jan-cerny
reviewed
Dec 4, 2023
There was a problem hiding this comment.
jcerny@fedora ~/work/git/scap-security-guide (pr/11332) $ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9 --remediate-using ansible rpm_verify_ownership
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/work/git/scap-security-guide/logs/rule-custom-2023-12-04-1343/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_rpm_verify_ownership
INFO - Script all_ownerships_ok.pass.sh using profile (all) OK
INFO - Script wrong_group_ownership.fail.sh using profile (all) OK
INFO - Script wrong_ownership.fail.sh using profile (all) OK
jcerny@fedora ~/work/git/scap-security-guide (pr/11333) $ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9 rpm_verify_ownership
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/work/git/scap-security-guide/logs/rule-custom-2023-12-04-1351/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_rpm_verify_ownership
INFO - Script all_ownerships_ok.pass.sh using profile (all) OK
INFO - Script wrong_group_ownership.fail.sh using profile (all) OK
INFO - Script wrong_ownership.fail.sh using profile (all) OK
jcerny@fedora ~/work/git/scap-security-guide (pr/11333) $
| This rule can take a long time to perform the check and might consume a considerable | ||
| amount of resources depending on the number of packages present on the system. It is not a | ||
| problem in most cases, but especially systems with a large number of installed packages | ||
| can be affected. See <code>https://access.redhat.com/articles/6999111</code>. |
There was a problem hiding this comment.
I think that the link to the article should be present only in RHEL products because users who aren't Red Hat customers can't access this page.
|
Code Climate has analyzed commit 4475fc9 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 58.5%. View more on Code Climate. |
|
/packit retest-failed |
jan-cerny
approved these changes
Dec 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
This PR extract the commits related to
rpm_verify_ownershipfrom #11319It:
Rationale:
Better description and awareness of possible performance issues in the rule.
Rule performance improvement.
Review Hints:
Automatus tests should be enough.