-
Notifications
You must be signed in to change notification settings - Fork 684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add checks for crontab and supporting cron directories #4858
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this patch, but there is a catch in using the CSV files when it generates a check for directories, see comment below.
rationale: |- | ||
Service configuration files enable or disable features of their respective services that if configured incorrectly | ||
can lead to insecure and vulnerable configurations. Therefore, service configuration files should be owned by the | ||
correct group to prevent unauthorized changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
again, the rule is about user
rationale: |- | ||
Service configuration files enable or disable features of their respective services that if configured incorrectly | ||
can lead to insecure and vulnerable configurations. Therefore, service configuration files should be owned by the | ||
correct group to prevent unauthorized changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
access rights instead of group
/etc,cron.allow,0,0,0644,cron_allow | ||
/etc,cron.d,0,0,0700,cron_d | ||
/etc,cron.daily,0,0,0700,cron_daily |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/etc/cron.daily
is a directory, but this way the templating system will generate a check for a file. To generate a check for directory please put the full path to the first column and leave the second column empty, e.g.:
/etc/cron.daily,,0,0,0700,cron_daily
Other entries in this CSV file which describe directories need to be changed the similar way as well. Also change the RHEL8 CSV.
# | ||
# profiles = xccdf_org.ssgproject.content_profile_C2S | ||
GROUP=ssgttgroup | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test scenario fails for me.
[jcerny@thinkpad scap-security-guide{cis_cron_perms}]$ python3 tests/test_suite.py rule --libvirt qemu:/
//system ssgts_rhel7 --datastream build/ssg-rhel7-ds-1.2.xml file_groupowner_cron_daily
Setting console output to log level INFO
INFO - The DataStream contains 2 Benchmarks
INFO - 0 - scap_org.open-scap_cref_ssg-rhel7-xccdf-1.2.xml
INFO - 1 - scap_org.open-scap_cref_ssg-rhel7-pcidss-xccdf-1.2.xml
INFO - Selected Benchmark is 0
INFO - To select a different Benchmark, use --xccdf-id-number option.
INFO - The base image option has not been specified, choosing libvirt-based test environment.
INFO - Logging into /home/jcerny/scap-security-guide/logs/rule-custom-2019-09-23-1517/test_suite.log
INFO - xccdf_org.ssgproject.content_rule_file_groupowner_cron_daily
ERROR - Script is_root.pass.sh using profile xccdf_org.ssgproject.content_profile_C2S found issue:
ERROR - Rule evaluation resulted in fail, instead of expected pass during initial stage
ERROR - The initial scan failed for rule 'xccdf_org.ssgproject.content_rule_file_groupowner_cron_daily'.
INFO - Script other_group.fail.sh using profile xccdf_org.ssgproject.content_profile_C2S OK
ERROR - Rule evaluation resulted in error, instead of expected fixed during remediation stage
ERROR - The remediation failed for rule 'xccdf_org.ssgproject.content_rule_file_groupowner_cron_daily'.
I suspect that it's related to the CSV file which leads to generating a different check than we want.
@jan-cerny Fixed up the wording on the rationales and the csvs. Hopefully that fixes your tests, I don't have a machine I can run them with at the moment, but I tested the ds output against my test vm and they were successful. Just looked at your changes in #4857, do you want me to go back and split up the rule keys prior to merge? |
@70k10 The new templating system is still under development, so I think you don't have to do anything now. Thank you for your contribution. |
Description:
Added checks for /etc/crontab and cron.{d,hourly,daily,weekly,monthly}
Rationale:
Completes the cron checks for C2S/CIS 5.1 section.