Review rpm_verify_hashes rule #11332

marcusburghardt · 2023-12-04T09:59:25Z

Description:

This PR extract the commits related to rpm_verify_hashes from #11319

It:

Updates the rule description and includes a warning about performance in some specific scenarios
Improve the OVAL readability
Optimize the Ansible remediation by removing unnecessary task
Include condition in Bash remediation to make it more robust.

Rationale:

Better description, remediation and awareness of possible performance issues in the rule.

Review Hints:

Automatus tests should be enough

Also update warning about high consume of system resources in some scenarios.

It was not identified opportunities to increase performance during the check. So the changes were limited to readability.

During the OVAL review of this rule it was noticed the Bash remediation was fragile for some valid cases. Included a condition to avoid errors.

Removed unnecessary task in Playbook and made the fact definition more flexible.

github-actions · 2023-12-04T10:01:07Z

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

github-actions · 2023-12-04T10:03:37Z

This datastream diff is auto generated by the check Compare DS/Generate Diff

Click here to see the full diff

New content has different text for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_hashes'.
--- xccdf_org.ssgproject.content_rule_rpm_verify_hashes
+++ xccdf_org.ssgproject.content_rule_rpm_verify_hashes
@@ -3,26 +3,33 @@
 Verify File Hashes with RPM
 
 [description]:
-Without cryptographic integrity protections, system
-executables and files can be altered by unauthorized users without
-detection.
-The RPM package management system can check the hashes of
-installed software packages, including many that are important to system
-security.
-To verify that the cryptographic hash of system files and commands matches vendor
-values, run the following command to list which files on the system
-have hashes that differ from what is expected by the RPM database:
+Without cryptographic integrity protections, system executables and files can be altered by
+unauthorized users without detection. The RPM package management system can check the hashes
+of installed software packages, including many that are important to system security.
+
+To verify that the cryptographic hash of system files and commands matches vendor values, run
+the following command to list which files on the system have hashes that differ from what is
+expected by the RPM database:
 $ rpm -Va --noconfig | grep '^..5'
-A "c" in the second column indicates that a file is a configuration file, which
-may appropriately be expected to change. If the file was not expected to
-change, investigate the cause of the change using audit logs or other means.
-The package can then be reinstalled to restore the file.
-Run the following command to determine which package owns the file:
+
+If the file was not expected to change, investigate the cause of the change using audit logs
+or other means. The package can then be reinstalled to restore the file. Run the following
+command to determine which package owns the file:
 $ rpm -qf FILENAME
+
 The package can be reinstalled from a yum repository using the command:
 $ sudo yum reinstall PACKAGENAME
+
 Alternatively, the package can be reinstalled from trusted media using the command:
 $ sudo rpm -Uvh PACKAGENAME
+
+[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]:
 11

OVAL for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_hashes' differs.
--- oval:ssg-rpm_verify_hashes:def:1
+++ oval:ssg-rpm_verify_hashes:def:1
@@ -1,2 +1,2 @@
 criteria AND
-criterion oval:ssg-test_files_fail_md5_hash:tst:1
+criterion oval:ssg-test_rpm_verify_hashes:tst:1

OCIL for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_hashes' differs.
--- ocil:ssg-rpm_verify_hashes_ocil:questionnaire:1
+++ ocil:ssg-rpm_verify_hashes_ocil:questionnaire:1
@@ -1,5 +1,5 @@
-The following command will list which files on the system
-have file hashes different from what is expected by the RPM database.
-$ rpm -Va --noconfig | awk '$1 ~ /..5/ && $2 != "c"'
+The following command will list which files on the system have file hashes different from what
+is expected by the RPM database.
+$ rpm -Va --noconfig | awk '$1 ~ /..5/'
       Is it the case that there is output?
       
bash remediation for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_hashes' differs.
--- xccdf_org.ssgproject.content_rule_rpm_verify_hashes
+++ xccdf_org.ssgproject.content_rule_rpm_verify_hashes
@@ -2,8 +2,11 @@
 # Find which files have incorrect hash (not in /etc, because of the system related config files) and then get files names
 files_with_incorrect_hash="$(rpm -Va --noconfig | grep -E '^..5' | awk '{print $NF}' )"
 
-# From files names get package names and change newline to space, because rpm writes each package to new line
-packages_to_reinstall="$(rpm -qf $files_with_incorrect_hash | tr '\n' ' ')"
+if [ -n "$files_with_incorrect_hash" ]; then
+    # From files names get package names and change newline to space, because rpm writes each package to new line
+    packages_to_reinstall="$(rpm -qf $files_with_incorrect_hash | tr '\n' ' ')"
 
-
-yum reinstall -y $packages_to_reinstall
+    
+    yum reinstall -y $packages_to_reinstall
+    
+fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_rpm_verify_hashes' differs.
--- xccdf_org.ssgproject.content_rule_rpm_verify_hashes
+++ xccdf_org.ssgproject.content_rule_rpm_verify_hashes
@@ -1,32 +1,7 @@
-- name: 'Set fact: Package manager reinstall command (dnf)'
-  set_fact:
-    package_manager_reinstall_cmd: dnf reinstall -y
-  when: ansible_distribution == "Fedora"
-  tags:
-  - CCE-80857-6
-  - CJIS-5.10.4.1
-  - NIST-800-171-3.3.8
-  - NIST-800-171-3.4.1
-  - NIST-800-53-AU-9(3)
-  - NIST-800-53-CM-6(c)
-  - NIST-800-53-CM-6(d)
-  - NIST-800-53-SI-7
-  - NIST-800-53-SI-7(1)
-  - NIST-800-53-SI-7(6)
-  - PCI-DSS-Req-11.5
-  - PCI-DSSv4-11.5.2
-  - high_complexity
-  - high_severity
-  - medium_disruption
-  - no_reboot_needed
-  - restrict_strategy
-  - rpm_verify_hashes
-
-- name: 'Set fact: Package manager reinstall command (yum)'
+- name: 'Set fact: Package manager reinstall command'
   set_fact:
     package_manager_reinstall_cmd: yum reinstall -y
-  when: (ansible_distribution == "RedHat" or ansible_distribution == "CentOS" or ansible_distribution
-    == "OracleLinux")
+  when: ansible_distribution in [ "Fedora", "RedHat", "CentOS", "OracleLinux" ]
   tags:
   - CCE-80857-6
   - CJIS-5.10.4.1

jan-cerny

Overall it looks good to me, it's a clever change reusing the filter.

The CI fail of Automatus on SLE15 is OK because there is an offending file.

But, the CI fail of Automatus on Fedora is caused by a hole in the test scenario linux_os/guide/system/software/integrity/software-integrity/rpm_verification/rpm_verify_hashes/tests/bad_document.fail.sh. I think it's worth fixing. Can you take look at the Fedora fail?

When executed locally, all tests are passing correctly.

jcerny@fedora ~/work/git/scap-security-guide (pr/11332) $ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9 --remediate-using ansible rpm_verify_hashes
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-1330/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_rpm_verify_hashes
INFO - Script bad_document.fail.sh using profile (all) OK
INFO - Script fresh_system.pass.sh using profile (all) OK
jcerny@fedora ~/work/git/scap-security-guide (pr/11332) $ python3 tests/automatus.py rule --libvirt qemu:///system ssgts_rhel9  rpm_verify_hashes
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-1333/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_rpm_verify_hashes
INFO - Script bad_document.fail.sh using profile (all) OK
INFO - Script fresh_system.pass.sh using profile (all) OK

...ide/system/software/integrity/software-integrity/rpm_verification/rpm_verify_hashes/rule.yml

jan-cerny · 2023-12-05T13:43:24Z

@marcusburghardt It didn't help. I have download the artifact from the Automatus Fedora job and the prescripts log still shows a peculiar message:

ssh -o Port=36279 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@localhost cd /root/ssgts/rpm_verify_hashes; SHARED=/root/ssgts/shared bash -x bad_document.fail.sh
STDERR: Warning: Permanently added '[localhost]:36279' (ECDSA) to the list of known hosts.
++ find /usr/share/doc -name TODO -o -name README
++ head -n 1
+ todo_file=
+ echo ' '
bad_document.fail.sh: line 7: $todo_file: ambiguous redirect

it looks like the variable is still empty.

Any suggestions?

marcusburghardt · 2023-12-05T13:59:25Z

@marcusburghardt It didn't help. I have download the artifact from the Automatus Fedora job and the prescripts log still shows a peculiar message:
ssh -o Port=36279 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@localhost cd /root/ssgts/rpm_verify_hashes; SHARED=/root/ssgts/shared bash -x bad_document.fail.sh
STDERR: Warning: Permanently added '[localhost]:36279' (ECDSA) to the list of known hosts.
++ find /usr/share/doc -name TODO -o -name README
++ head -n 1
+ todo_file=
+ echo ' '
bad_document.fail.sh: line 7: $todo_file: ambiguous redirect
it looks like the variable is still empty.

Any suggestions?

I would like to avoid installing packages but I think this will be necessary for this test case. Let me try one here.

marcusburghardt · 2023-12-05T14:25:43Z

@marcusburghardt It didn't help. I have download the artifact from the Automatus Fedora job and the prescripts log still shows a peculiar message:
ssh -o Port=36279 -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null root@localhost cd /root/ssgts/rpm_verify_hashes; SHARED=/root/ssgts/shared bash -x bad_document.fail.sh
STDERR: Warning: Permanently added '[localhost]:36279' (ECDSA) to the list of known hosts.
++ find /usr/share/doc -name TODO -o -name README
++ head -n 1
+ todo_file=
+ echo ' '
bad_document.fail.sh: line 7: $todo_file: ambiguous redirect
it looks like the variable is still empty.
Any suggestions?
I would like to avoid installing packages but I think this will be necessary for this test case. Let me try one here.

It didn't work. I will try to reproduce it locally and find a way to make this test scenario more robust.

marcusburghardt · 2023-12-06T10:34:25Z

It should be ok now @jan-cerny

jan-cerny · 2023-12-06T12:19:51Z

/packit build

jan-cerny · 2023-12-06T13:24:41Z

/packit reset

jan-cerny · 2023-12-06T13:24:52Z

/packit build

The intention is to modify the hash of a file managed by rpm package. The test was searching for specific file name in /usr/share/doc to be modified but some systems might not have the files assumed to be present. Therefore, the test scenario was updated to modify the ls" command, which is present even in minimal installations and a small modification for testing purposes is not harmfull for the system.

codeclimate · 2023-12-06T14:16:21Z

Code Climate has analyzed commit 50f8a8a 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.

jan-cerny · 2023-12-07T10:42:44Z

/packit retest-failed

marcusburghardt added 4 commits December 4, 2023 10:48

Improve rule description in rpm_verify_hashes

4a3810d

Also update warning about high consume of system resources in some scenarios.

Improved OVAL readability in rpm_verify_hashes

8ccbd63

It was not identified opportunities to increase performance during the check. So the changes were limited to readability.

Minor improvements in Bash remediation for rpm_verify_hashes

fdd108d

During the OVAL review of this rule it was noticed the Bash remediation was fragile for some valid cases. Included a condition to avoid errors.

Minor improvements in Ansible remediation for rpm_verify_hashes

96f119c

Removed unnecessary task in Playbook and made the fact definition more flexible.

marcusburghardt added the Update Rule Issues or pull requests related to Rules updates. label Dec 4, 2023

marcusburghardt added this to the 0.1.72 milestone Dec 4, 2023

marcusburghardt requested a review from jan-cerny December 4, 2023 09:59

marcusburghardt mentioned this pull request Dec 4, 2023

Improve Performance on rules probing the whole file system #11319

Merged

10 tasks

jan-cerny self-assigned this Dec 4, 2023

jan-cerny requested changes Dec 4, 2023

View reviewed changes

...ide/system/software/integrity/software-integrity/rpm_verification/rpm_verify_hashes/rule.yml Outdated Show resolved Hide resolved

Show rhel article in warning only to rhel products

93d720a

marcusburghardt requested a review from jan-cerny December 5, 2023 12:50

marcusburghardt force-pushed the rpm_verify_hashes_review branch from 1a91385 to 5e30f5b Compare December 5, 2023 14:13

marcusburghardt force-pushed the rpm_verify_hashes_review branch from 5e30f5b to ba2a3c2 Compare December 6, 2023 10:24

marcusburghardt force-pushed the rpm_verify_hashes_review branch from ba2a3c2 to 50f8a8a Compare December 6, 2023 14:03

jan-cerny approved these changes Dec 7, 2023

View reviewed changes

jan-cerny merged commit d868537 into ComplianceAsCode:master Dec 7, 2023
37 of 38 checks passed

marcusburghardt deleted the rpm_verify_hashes_review branch December 7, 2023 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Review rpm_verify_hashes rule #11332

Review rpm_verify_hashes rule #11332

marcusburghardt commented Dec 4, 2023

github-actions bot commented Dec 4, 2023

github-actions bot commented Dec 4, 2023 •

edited

Loading

jan-cerny left a comment

jan-cerny commented Dec 5, 2023

marcusburghardt commented Dec 5, 2023

marcusburghardt commented Dec 5, 2023

marcusburghardt commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

codeclimate bot commented Dec 6, 2023

jan-cerny commented Dec 7, 2023

Review rpm_verify_hashes rule #11332

Review rpm_verify_hashes rule #11332

Conversation

marcusburghardt commented Dec 4, 2023

Description:

Rationale:

Review Hints:

github-actions bot commented Dec 4, 2023

github-actions bot commented Dec 4, 2023 • edited Loading

jan-cerny left a comment

Choose a reason for hiding this comment

jan-cerny commented Dec 5, 2023

marcusburghardt commented Dec 5, 2023

marcusburghardt commented Dec 5, 2023

marcusburghardt commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

jan-cerny commented Dec 6, 2023

codeclimate bot commented Dec 6, 2023

jan-cerny commented Dec 7, 2023

github-actions bot commented Dec 4, 2023 •

edited

Loading