Skip to content
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

Creation of Australian ISM 'Official' RHEL 8 profile #5861

Merged
merged 5 commits into from
Jul 1, 2020

Conversation

wcushen
Copy link
Contributor

@wcushen wcushen commented Jun 22, 2020

Description:

Pull Request to merge ISM Official YAML profile (RHEL 8) as part of the work done by Red Hat ANZ to extend ACSC Essential Eight profile for customers in the region.

This profile represents the first of four 'applicability markings' (see page 2) set by the Australian Attorney-General’s Department. This profile represents the 'OFFICIAL' baseline.

https://www.cyber.gov.au/sites/default/files/2019-03/ISM_01_Cyber_Security_Framework.pdf

Rationale:

New SCAP profile. No amendments have been made to any existing codebase.

@openscap-ci
Copy link
Collaborator

Can one of the admins verify this patch?

1 similar comment
@openscap-ci
Copy link
Collaborator

Can one of the admins verify this patch?

@openshift-ci-robot
Copy link
Collaborator

Hi @wcushen. 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.

@openshift-ci-robot openshift-ci-robot added the needs-ok-to-test Used by openshift-ci bot. label Jun 22, 2020
@wcushen wcushen closed this Jun 22, 2020
@wcushen wcushen reopened this Jun 22, 2020
@wcushen wcushen force-pushed the master branch 3 times, most recently from 91bb82f to 69f934e Compare June 22, 2020 04:39
@jan-cerny
Copy link
Collaborator

@openscap-ci ok to test

@jan-cerny
Copy link
Collaborator

@openshift-ci-robot
/ok-to-test

@openshift-ci-robot openshift-ci-robot added ok-to-test Used by openshift-ci bot. and removed needs-ok-to-test Used by openshift-ci bot. labels Jun 22, 2020
- accounts_password_minlen_login_defs
- accounts_password_pam_minclass
- accounts_password_pam_minlen
- accounts_password_pam_pwquality
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is similar, Th rule accounts_password_pam_pwquality doesn't exist, it just a check that's embedded in other accounts_password_pam_.* rules.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## Secure Shell access
## Identifiers 1506 / 1449 / 0487
- sshd_version_equal_or_higher_than_74
- sshd_allow_only_protocol2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rule sshd_version_equal_or_higher_than_74 doesn't exist, it's a mere OVAL check that is a part of rule sshd_allow_only_protocol2. I think that it's enough to keep selected only rule sshd_allow_only_protocol2. If you need for some reason to have a separate rule for the SSH version you will have to create rule directory and rule.yml which uses this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If sshd_allow_only_protocol2 covers https://www.tenable.com/plugins/nessus/93194, then free to remove.

Removed here wcushen@a50c2a6

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong, but I believe that there is no longer a way to configure Protocol version 2 in RHEL8+

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redhatrises

I have seen it cited in a few RHEL 8 profiles:

RH CCP:

- sshd_allow_only_protocol2

CJIS:
- sshd_allow_only_protocol2

HIPAA:
- sshd_allow_only_protocol2

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I am wrong, but I believe that there is no longer a way to configure Protocol version 2 in RHEL8+

You are correct.

The rule has a conditional on the SSH version, if OpenSSH is 7.3 or older, it checks for the configuration: https://github.com/ComplianceAsCode/content/blob/master/linux_os/guide/services/ssh/ssh_server/sshd_allow_only_protocol2/oval/shared.xml#L22
For RHEL8+, the rule become just a check for OpenSSH version.

Copy link
Contributor Author

@wcushen wcushen Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuumasato So can omit if RHEL 8+ has rebased OpenSSH version 7.8p1 (i.e. no longer supports protocol 1)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wcushen Yeah, RHEL8+ OpenSSH packages don't provide choice to switch to protocol 1.

If the policy explicitly calls out a check for protocol 2, the rule should probably be there.
Ultimately it is up to you to decide whether to omit the rule or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yuumasato If it's cited in other RHEL 8 profile (CJIS, HIPAA, RH CCP) it'd be good to keep to maintain consistency - even it does just represent a check of the version.

@vojtapolasek vojtapolasek self-assigned this Jun 22, 2020
## Identifiers 1418
- package_usbguard_installed
- service_usbguard_enabled
- usbguard_rules_not_empty_not_missing
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also not a rule, it is just a check used in different usbguard rules. Please see rules in the services/usbguard directory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

- rsyslog_remote_loghost
- rsyslog_remote_tls
- rsyslog_remote_tls_cacert
- service_ntpd_enabled
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supported ntp software on rhel8 is Chrony afaik. Could you change selections? We have some rules for Chrony, see the services/ntp folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @vojtapolasek. Swapped out ntpd for Chrony wcushen@d713d44

## Identifiers 0584 / 0582 / 0585 / 0586 / 0846 / 0957
- display_login_attempts
- sebool_auditadm_exec_content
- audit_rules_privileged_commands_pam_timestamp_check
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't need this explicit rule if you use audit_rules_privileged_commands. Audit_rules_privileged commands is a dynamic rule which checks for privileged commands on the system, see the description. Therefore it the binary pam_timestamp_check is installed, it will be included. @yuumasato am I right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, audit_rules_privileged_commands covers all setuid and setgid binaries, and checks on rpm database if it is expected.
While audit_rules_privileged_commands_pam_timestamp_check is specific for pam_timestamp_check only.

Copy link
Contributor Author

@wcushen wcushen Jun 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vojtapolasek @yuumasato Removed pam_timestamp_check wcushen@e9cbc6a

@vojtapolasek
Copy link
Collaborator

Thank you very much for this contribution. I did some testing.
I used upstream test suite to check remediation for whole profile through Bash and Ansible.
If not stated specifically, rules have remediation but are still failing. It could be problem with the remediation or some other problem, e.g. ordering of rules.
Here are results:
bash:
153 passed
10 failed
1 other
failing rules are
xccdf_org.ssgproject.content_rule_force_opensc_card_drivers
xccdf_org.ssgproject.content_rule_configure_opensc_card_drivers
xccdf_org.ssgproject.content_rule_enable_fips_mode
xccdf_org.ssgproject.content_rule_rsyslog_remote_tls - this rule is not expected to have automatic remediation
xccdf_org.ssgproject.content_rule_rsyslog_remote_tls_cacert - this rule is not expected to have automatic remediation
xccdf_org.ssgproject.content_rule_network_ipv6_static_address - this rule is not expected to have automatic remediation
xccdf_org.ssgproject.content_rule_service_usbguard_enabled
xccdf_org.ssgproject.content_rule_enable_ldap_client - this rule is lacking remediation
xccdf_org.ssgproject.content_rule_ntpd_specify_remote_server - can be switched for a different rule which passes, see comments
xccdf_org.ssgproject.content_rule_ntpd_specify_multiple_servers - can be switched for a different rule which passes, see comments

ansible:
145 passed
failing rules are:
xccdf_org.ssgproject.content_rule_force_opensc_card_drivers
xccdf_org.ssgproject.content_rule_configure_opensc_card_drivers
xccdf_org.ssgproject.content_rule_audit_rules_privileged_commands - will be passing if the rule xccdf_org.ssgproject.content_rule_audit_rules_privileged_commands_pam_timestamp_check is dropped, see comments
xccdf_org.ssgproject.content_rule_audit_rules_privileged_commands_pam_timestamp_chec - should be dropped, see comments
xccdf_org.ssgproject.content_rule_audit_rules_unsuccessful_file_modification
xccdf_org.ssgproject.content_rule_audit_rules_usergroup_modification
xccdf_org.ssgproject.content_rule_enable_fips_mode
xccdf_org.ssgproject.content_rule_rsyslog_remote_tls - is not expected to be remediated automatically
xccdf_org.ssgproject.content_rule_rsyslog_remote_tls_cacert - is not expected to be remediated automatically
xccdf_org.ssgproject.content_rule_mount_option_dev_shm_noexec
xccdf_org.ssgproject.content_rule_file_permissions_unauthorized_world_writable - not expected to be remediated automatically
xccdf_org.ssgproject.content_rule_network_ipv6_static_address - not expected to be remediated automatically
xccdf_org.ssgproject.content_rule_network_nmcli_permissions
xccdf_org.ssgproject.content_rule_service_usbguard_enabled
xccdf_org.ssgproject.content_rule_enable_ldap_client - lacking remediation
xccdf_org.ssgproject.content_rule_sssd_enable_smartcards
xccdf_org.ssgproject.content_rule_ntpd_specify_remote_server - can be switched for a rule which will pass, see comments
xccdf_org.ssgproject.content_rule_ntpd_specify_multiple_servers - same as above

As I said, there are still some suggestions which will improve the profile. I am also waiting for #5870 to be reviewed and merged, as currently the profile Ansible playbook ends with a fatal failure.
Please review suggestions. I am fine with merging the profile, probably later gradually fixing broken remediations. Thank you once again for this new profile.

@redhatrises
Copy link
Contributor

/retest

@wcushen
Copy link
Contributor Author

wcushen commented Jun 23, 2020

Thank you very much for this contribution. I did some testing.
I used upstream test suite to check remediation for whole profile through Bash and Ansible.
If not stated specifically, rules have remediation but are still failing. It could be problem with the remediation or some other problem, e.g. ordering of rules.

@vojtapolasek Thank you so much for your review! I've committed the suggestions.
That is quite a lot of failures however, but I assume controls such as enable_fips_mode and force_opensc_card_drivers would be widely used across other profiles?


## Authentication hardening
## Identifiers 1546 / 0974 / 1173 / 1504 / 1505 / 1401 / 1559 / 1560
## 1561 / 0421 / 1557 / 0422 / 1558 / 1403 / 0431
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wcushen these identifiers need to be added to the rules themselves vs the profile via new references for e8 and ism.
I can do this in a follow on PR if you prefer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@redhatrises Yes noted. The thinking with @shaneboulden was that once merged, we would go back over each control and add the ISM identifiers in the main content.
So I think a follow on PR is preferred here.

@vojtapolasek
Copy link
Collaborator

Can you please rebase on current master? I noticed that some errors, e.g. problem with Ansible, is already fixed. I will do another round of tests based on master branch, we might get much better results.

@wcushen
Copy link
Contributor Author

wcushen commented Jun 24, 2020

Can you please rebase on current master? I noticed that some errors, e.g. problem with Ansible, is already fixed. I will do another round of tests based on master branch, we might get much better results.
@vojtapolasek That should be done now.

## ASD Approved Cryptopgraphic Algorithims
## Identifiers 1446
- enable_dracut_fips_module
- enable_fips_mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this rule to pass, you have to also include:
- var_system_crypto_policy=fips_ospp
- configure_crypto_policy
This will configure system-vide crypto policy to "fips". Make sure that this IS what you want.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated via wcushen@c05588a
I've opted for var_system_crypto_policy=fips not fips_ospp. Is that OK @vojtapolasek?

Copy link
Contributor Author

@wcushen wcushen Jun 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated again to wcushen@dbc698d to align with default_nosha1 set Essential Eight RHEL 8

- var_system_crypto_policy=default_nosha1
I thought as it extends this profile (E8) that that var would have been picked up?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were thinking right, the variable will be picked up from the parent profile. However, be aware that the rule enable_fips_mode will fail if the policy is set to default-nosha1. That's stems from its definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

## Endpoint device control software
## Identifiers 1418
- package_usbguard_installed
- service_usbguard_enabled
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason the service fails to start after remediation of the profile. It seems like problem on usbguard side. I will report a bug after we merge this.

@wcushen
Copy link
Contributor Author

wcushen commented Jun 24, 2020

@vojtapolasek I noticed an error during one of the builds the other day regarding the presence of configure_firewalld_rate_limiting. Is this a valid rule for RHEL 8?

I thought maybe it requires a var but it is present in RHEL7 STIG without one.

https://github.com/wcushen/content/blob/master/rhel8/profiles/ism_o.profile#L34

@mildas
Copy link
Contributor

mildas commented Jun 25, 2020

Changes identified:
Profile ism_o on rhel8:
 Newly added profile.

Recommended tests to execute:
 build_product rhel8
 tests/test_suite.py profile --libvirt qemu:///system test-suite-vm --datastream build/ssg-rhel8-ds.xml ism_o

@vojtapolasek
Copy link
Collaborator

Thank you for all the changes. The PR looks good to me and I am merging it. Actually some failing rules might have been caused by my testing system, for example I no longer see the service_usbguard_enabled failing. Feel free to fill upstream issues for rules which are failing and we will do our best to investigate them.

@vojtapolasek vojtapolasek merged commit 39a9663 into ComplianceAsCode:master Jul 1, 2020
@yuumasato yuumasato added this to the 0.1.51 milestone Jul 2, 2020
@yuumasato yuumasato added the Highlight This PR/Issue should make it to the featured changelog. label Jul 2, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Highlight This PR/Issue should make it to the featured changelog. ok-to-test Used by openshift-ci bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants