Use bash_package_*

[maage]

Description:

Fix aide_check_audit_tools bug where it did not handle suffixes of prefixes and had bad test, found when testing this.

Use bash_package_install or bash_package_remove or # packages = to manage packages, not plain shell commands.

Add some missing packages seen when test system was bare bones.

Rationale:

Simplifies code. Makes it more product independent.

Review Hints:

Has potentially functionality changes. Unfortunately somewhat all over. Mainly should check aide.

[openshift-ci]

Hi @maage. Thanks for your PR.

I'm waiting for a ComplianceAsCode member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[github-actions]

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]

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

Click here to see the full diff

bash remediation for rule 'xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking' differs.
--- xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking
+++ xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking
@@ -3,6 +3,9 @@
 
 if ! rpm -q --quiet "aide" ; then
 yum install -y "aide"
+fi
+if ! rpm -q --quiet "crontabs" ; then
+ yum install -y "crontabs"
 fi
 
 if ! grep -q "/usr/sbin/aide --check" /etc/crontab ; then

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking' differs.
--- xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking
+++ xccdf_org.ssgproject.content_rule_aide_periodic_cron_checking
@@ -1,9 +1,9 @@
 - name: Ensure AIDE is installed
 package:
- name: '{{ item }}'
+ name:
+ - aide
+ - crontabs
 state: present
- with_items:
- - aide
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
 tags:
 - CCE-80676-0

bash remediation for rule 'xccdf_org.ssgproject.content_rule_aide_scan_notification' differs.
--- xccdf_org.ssgproject.content_rule_aide_scan_notification
+++ xccdf_org.ssgproject.content_rule_aide_scan_notification
@@ -3,6 +3,9 @@
 
 if ! rpm -q --quiet "aide" ; then
 yum install -y "aide"
+fi
+if ! rpm -q --quiet "crontabs" ; then
+ yum install -y "crontabs"
 fi
 var_aide_scan_notification_email=''
 

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_aide_scan_notification' differs.
--- xccdf_org.ssgproject.content_rule_aide_scan_notification
+++ xccdf_org.ssgproject.content_rule_aide_scan_notification
@@ -6,10 +6,10 @@
 
 - name: Ensure AIDE is installed
 package:
- name: '{{ item }}'
+ name:
+ - aide
+ - crontabs
 state: present
- with_items:
- - aide
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
 tags:
 - CCE-82891-3

[codeclimate]

Code Climate has analyzed commit 7f58b56 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 52.4% (0.0% change).

View more on Code Climate.

[maage]

Again these are misleading, and are not caused by this PR

ERROR - Rule 'xccdf_org.ssgproject.content_rule_smartcard_configure_cert_checking' isn't present in benchmark 'xccdf_org.ssgproject.content_benchmark_RHEL-8' in '/tmp/ssgts-ds-tamw53zo'
ERROR - Rule 'xccdf_org.ssgproject.content_rule_auditd_offload_logs' isn't present in benchmark 'xccdf_org.ssgproject.content_benchmark_RHEL-8' in '/tmp/ssgts-ds-tamw53zo'
WARNING - Script ntp_multiple_misconfigured.fail.sh is not applicable on given platform

[marcusburghardt]

There are some errors with the aide_check_audit_tools rule when remediating RHEL (maybe also other distros) VMs:

./tests/automatus.py rule --libvirt qemu:///session rhel88 --datastream build/ssg-rhel8-ds.xml --dontclean --remediate-using bash aide_check_audit_tools
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - xccdf_org.ssgproject.content_rule_aide_check_audit_tools
INFO - Script correct.pass.sh using profile (all) OK
INFO - Script correct_with_selinux.pass.sh using profile (all) OK
INFO - Script extra_suffix.fail.sh using profile (all) OK
ERROR - Bash remediation for rule xccdf_org.ssgproject.content_rule_aide_check_audit_tools has exited with these errors:
+ echo 'Remediating rule 1/1: '\''xccdf_org.ssgproject.content_rule_aide_check_audit_tools'\'''
Remediating rule 1/1: 'xccdf_org.ssgproject.content_rule_aide_check_audit_tools'
+ '[' '!' -f /.dockerenv ']'
+ '[' '!' -f /run/.containerenv ']'
+ rpm -q --quiet aide
+ grep -i '^.*/usr/sbin/auditctl.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/auditctl.*#/usr/sbin/auditctl p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 75: unterminated `s' command
+ grep -i '^.*/usr/sbin/auditd.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/auditd.*#/usr/sbin/auditd p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 71: unterminated `s' command
+ grep -i '^.*/usr/sbin/ausearch.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/ausearch.*#/usr/sbin/ausearch p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 75: unterminated `s' command
+ grep -i '^.*/usr/sbin/aureport.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/aureport.*#/usr/sbin/aureport p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 75: unterminated `s' command
+ grep -i '^.*/usr/sbin/autrace.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/autrace.*#/usr/sbin/autrace p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 73: unterminated `s' command
+ grep -i '^.*/usr/sbin/augenrules.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/augenrules.*#/usr/sbin/augenrules p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 79: unterminated `s' command
+ grep -i '^.*/usr/sbin/rsyslogd.*$' /etc/aide.conf
+ sed -i 's#.*/usr/sbin/rsyslogd.*#/usr/sbin/rsyslogd p+i+n+u+g+s+b+acl+xattrs+sha512
#' /etc/aide.conf
sed: -e expression #1, char 75: unterminated `s' command

ERROR - The remediation failed for rule 'xccdf_org.ssgproject.content_rule_aide_check_audit_tools'.
INFO - Script not_config.fail.sh using profile (all) OK

Mon May 15 09:30:10 AM CEST 2023 - rhel8 - aide_check_audit_tools - ansible VM
./tests/automatus.py rule --libvirt qemu:///session rhel88 --datastream build/ssg-rhel8-ds.xml --dontclean --remediate-using ansible aide_check_audit_tools
Setting console output to log level INFO
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - xccdf_org.ssgproject.content_rule_aide_check_audit_tools
INFO - Script correct.pass.sh using profile (all) OK
INFO - Script correct_with_selinux.pass.sh using profile (all) OK
INFO - Script extra_suffix.fail.sh using profile (all) OK
ERROR - Rule evaluation resulted in fail, instead of expected pass during final stage 
ERROR - The check after remediation failed for rule 'xccdf_org.ssgproject.content_rule_aide_check_audit_tools'.
INFO - Script not_config.fail.sh using profile (all) OK
ERROR - Rule evaluation resulted in fail, instead of expected pass during final stage 
ERROR - The check after remediation failed for rule 'xccdf_org.ssgproject.content_rule_aide_check_audit_tools'.

@maage , could you take a look on this, please?

[maage]

There are some errors with the aide_check_audit_tools rule when remediating RHEL (maybe also other distros) VMs:
...
@maage , could you take a look on this, please?

#10578

There is also some other cases where newlines are added to values, see #10534. So generally this issue should be handled somehow or failed fast, and not like sed: -e expression #1, char 75: unterminated 's' command when running remediation.

Slightly off topic rant follows,

But somewhat better fix would be to improve bash_replace_or_append to handle this kind of key is regex (no stripped_key). That would create simple solution, but not right.

But key should not be too much regexp so that it can be remediated. Now that I'm thinking about how to bypass the rule and there are definitely possiblities. https://aide.github.io/doc/#config "Understanding AIDE rule matching".
Is it possible to build a regexp that matches that matches all regexps that matches a certaing filename, including itself? And what about ordering?
For example now what would happen with these,

...
/usr/sbin/auditd p+i+n+u+g+s+b+acl+selinux+xattrs+sha512
!/
# EOF

Something like:

182 /var/lib/aide/aide.db.new.gz

182 is size of database of whole system in bytes.

So only way I can see to make this remediation to definitely work is to generate a config in simple manner, ask aide what it thinks about it and then based on that and if it does something else, reject the config. Run aide -u -c newconfig, check db and collect all the right values for files interested and ensure db has all matched. I guess this could be easily done with aide itself by having very limited but correct static config and then compare result and fail remediation if results do not match.

But if configuration is right from OVAL check part, then no remediation is triggered. Rule tests should have another way to check the status of system. Currently most remediations do not check that after configuration system actually does what is tried to configure. This is like adding a lock to a door and then checking the lock, but not checking if the door opens when the lock is locked. Many rules focus only on right looking configuration, but forget to check that system works to achieve what rule / control behind tries to achieve.

If this kind of solution is not possible, then there should be backends for any complex configuration language system. If configuration needs EBNF syntax (sudo) or external manual where there is need for pseudocode (aide), is generally way beyond any way to definitely handle with one liner regexps.

I guess only way to definitely handle this is just limit what kind of configurations are accepted. But usually that just weeds out so-called real-world cases. For example for aide, accept only suffix regexps and only limited variants. Define only one defined order.

[marcusburghardt]

There are some errors with the aide_check_audit_tools rule when remediating RHEL (maybe also other distros) VMs:
...
@maage , could you take a look on this, please?

#10578

Thanks for the #10578, it fixes the mentioned issue.

There is also some other cases where newlines are added to values, see #10534. So generally this issue should be handled somehow or failed fast, and not like sed: -e expression #1, char 75: unterminated 's' command when running remediation.

I agree. I see you are already working on something related to this in #10574. Thanks for that.

Slightly off topic rant follows,

But somewhat better fix would be to improve bash_replace_or_append to handle this kind of key is regex (no stripped_key). That would create simple solution, but not right.

But key should not be too much regexp so that it can be remediated. Now that I'm thinking about how to bypass the rule and there are definitely possiblities. https://aide.github.io/doc/#config "Understanding AIDE rule matching". Is it possible to build a regexp that matches that matches all regexps that matches a certaing filename, including itself? And what about ordering? For example now what would happen with these,

...
/usr/sbin/auditd p+i+n+u+g+s+b+acl+selinux+xattrs+sha512
!/
# EOF

Something like:

182 /var/lib/aide/aide.db.new.gz

182 is size of database of whole system in bytes.

So only way I can see to make this remediation to definitely work is to generate a config in simple manner, ask aide what it thinks about it and then based on that and if it does something else, reject the config. Run aide -u -c newconfig, check db and collect all the right values for files interested and ensure db has all matched. I guess this could be easily done with aide itself by having very limited but correct static config and then compare result and fail remediation if results do not match.

But if configuration is right from OVAL check part, then no remediation is triggered. Rule tests should have another way to check the status of system. Currently most remediations do not check that after configuration system actually does what is tried to configure. This is like adding a lock to a door and then checking the lock, but not checking if the door opens when the lock is locked. Many rules focus only on right looking configuration, but forget to check that system works to achieve what rule / control behind tries to achieve.

Indeed, the rules are intended to check the configuration but not its functionality. It would be tricky and likely not feasible for many rules. So, in general, benchmarks and vendors do these tests to determine the proper configuration to be checked in order to achieve a Benchmark requirement. Usually when something changes in the system, the flow is to interact with the Benchmarks and propose the changes.

If this kind of solution is not possible, then there should be backends for any complex configuration language system. If configuration needs EBNF syntax (sudo) or external manual where there is need for pseudocode (aide), is generally way beyond any way to definitely handle with one liner regexps.

I guess only way to definitely handle this is just limit what kind of configurations are accepted. But usually that just weeds out so-called real-world cases. For example for aide, accept only suffix regexps and only limited variants. Define only one defined order.

Thanks for this analysis regarding AIDE. It would be great if you file an issue with this information so we can collaborate investigating better solutions.

[marcusburghardt]

Thanks for these improvements.

[svet-se]

Hey, @maage I've noticed that when you install the crontabs package you do not check the product, so at the moment it's not working for SLE, I'm not sure for RedHat, so maybe this merge should be reverted. @marcusburghardt please advise.

[maage]

@svet-se Could it be possible to implement platform_package_overrides for the crontab package? For example file_cron_deny_not_exist, has # packages = crontabs in tests and SLE in prodtype. Does it work?

I think issue here is that platform_package_overrides is not implemented bash_package_install and there is no ansible_package_install. My conversation starter at: #10626

[svet-se]

Thanks @maage

I believe it's possible to fix that, but maybe it will be much better when making changes and targeting the specific platform to use some checks like this:

• name: Set cron package name - RedHat
  set_fact:
  cron_pkg_name: cronie
  when: ansible_os_family == "RedHat" or ansible_os_family == "Suse"

IMHO it's better to be more explicit than implicit.

Best regards and have a nice day

[dodys]

@maage you also broke it for Ubuntu in both bash (not really broke it but throwing error message) and ansible (see #10725), there's no crontabs package in ubuntu.
Also if we already have the 'Install cron' in ansible, why would you need the crontabs in the "Ensure aide is installed"?

This PR should have triggered reviews from other distros, this was definitely a mistake.
@marcusburghardt @Mab879 ^

[marcusburghardt]

@maage you also broke it for Ubuntu in both bash (not really broke it but throwing error message) and ansible (see #10725), there's no crontabs package in ubuntu. Also if we already have the 'Install cron' in ansible, why would you need the crontabs in the "Ensure aide is installed"?

This PR should have triggered reviews from other distros, this was definitely a mistake. @marcusburghardt @Mab879 ^

Thanks for the PR proposing to reverting the commit @dodys . We can review the related rules separately.