correct update-crypto-policies --set is not idempotent

[jackmanb1985]

Description:

update-crypto-policies --set is not idempotent and will execute on subsequent runs of Ansible. Check and compare the current runtime value of update-crypto-policies by invoking update-crypto-policies --show and registering its output as a variable which can be used as a conditional on the subsequent update-crypto-policies --set

Rationale:

Ansible should be idempotent by providing a conditional state to compare against.

[openshift-ci[bot]]

Hi @jackmanb1985. 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[bot]]

Start a new ephemeral environment with changes proposed in this pull request:

[github-actions[bot]]

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

Click here to see the full diff

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_enable_fips_mode' differs:
--- old datastream
+++ new datastream
@@ -69,8 +69,9 @@
 - reboot_required
 - restrict_strategy
 
-- name: Verify that Crypto Policy is Set (runtime)
- command: /usr/bin/update-crypto-policies --set {{ var_system_crypto_policy }}
+- name: Register Crypto Policy value (runtime)
+ command: /usr/bin/update-crypto-policies --show
+ register: update_crypto_policies_show
 when: ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
 tags:
 - CCE-80942-6
@@ -87,3 +88,24 @@
 - medium_disruption
 - reboot_required
 - restrict_strategy
+
+- name: Verify that Crypto Policy is Set (runtime)
+ command: /usr/bin/update-crypto-policies --set {{ var_system_crypto_policy }}
+ when:
+ - ansible_virtualization_type not in ["docker", "lxc", "openvz", "podman", "container"]
+ - update_crypto_policies_show.stdout != var_system_crypto_policy
+ tags:
+ - CCE-80942-6
+ - DISA-STIG-RHEL-08-010020
+ - NIST-800-53-CM-6(a)
+ - NIST-800-53-IA-7
+ - NIST-800-53-SC-12
+ - NIST-800-53-SC-12(2)
+ - NIST-800-53-SC-12(3)
+ - NIST-800-53-SC-13
+ - enable_fips_mode
+ - high_severity
+ - medium_complexity
+ - medium_disruption
+ - reboot_required
+ - restrict_strategy

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_configure_crypto_policy' differs:
--- old datastream
+++ new datastream
@@ -27,8 +27,9 @@
 - no_reboot_needed
 - restrict_strategy
 
-- name: Verify that Crypto Policy is Set (runtime)
- command: /usr/bin/update-crypto-policies --set {{ var_system_crypto_policy }}
+- name: Register Crypto Policy value (runtime)
+ command: /usr/bin/update-crypto-policies --show
+ register: update_crypto_policies_show
 tags:
 - CCE-80935-0
 - DISA-STIG-RHEL-08-010020
@@ -45,3 +46,23 @@
 - low_disruption
 - no_reboot_needed
 - restrict_strategy
+
+- name: Verify that Crypto Policy is Set (runtime)
+ command: /usr/bin/update-crypto-policies --set {{ var_system_crypto_policy }}
+ when: update_crypto_policies_show.stdout != var_system_crypto_policy
+ tags:
+ - CCE-80935-0
+ - DISA-STIG-RHEL-08-010020
+ - NIST-800-53-AC-17(2)
+ - NIST-800-53-AC-17(a)
+ - NIST-800-53-CM-6(a)
+ - NIST-800-53-MA-4(6)
+ - NIST-800-53-SC-12(2)
+ - NIST-800-53-SC-12(3)
+ - NIST-800-53-SC-13
+ - configure_crypto_policy
+ - high_severity
+ - low_complexity
+ - low_disruption
+ - no_reboot_needed
+ - restrict_strategy

[Mab879]

/ok-to-test

[jackmanb1985]

/retest

[jan-cerny]

Hi, please remove all "Merge ..." commits from this PR, the "Merge ..." commits can't be accepted.

[jan-cerny]

Thanks for the rebase, but the test scenarios are still failing a lot, the test run still has the same output as in the comment above.

For example, the test scenario config_newer_than_current.fail.sh modifies mtime on /etc/crypto-policies/config, but the new Ansible remediation probably doesn't regenerate the configuration, therefore the final stage scan fails on the crypto_policies_updated OVAL test which tests whether the mtime of /etc/crypto-policies/config is less than or equal the mtime of /etc/crypto-policies/state/current. You will probably need to ensure the update-crypto-policies command is exeucted if this file has been modified.

Another example is the test scenario missing_nss_config.fail.sh which removes the file /etc/crypto-policies/back-ends/nss.config and then expects that rule will fail and remediation will regenerate this file. But on the final scan results you can see that the test for presence of this file failed, that means that the remediation didn't create it.

Similarly, the test missing_policy_file.fail.sh removes /etc/crypto-policies/state/current but the remediation didn't create it during the scan.

The pattern I observe is that you can't rely solely on the update-crypto-policies --show to determine whether to set a crypto policy or do nothing. You should follow all the branches of the associated OVAL check https://github.com/ComplianceAsCode/content/blob/08436ce586a70cfaaa22411167d9cdf80fdcb182/linux_os/guide/system/software/integrity/crypto/configure_crypto_policy/oval/shared.xml to figure out how to check for a correct state that requires no action and I guess execute the action in every other situation.

@yuumasato Do you have any additional information?

Also, I have noticed that the corresponding Bash remediation also just runs update-crypto-policies without any preliminary checks. And we usually keep Ansible in line with Bash.

[yuumasato]

Maybe update-crypto-policyes --is-applied could be used?

After modifying the config manually, I get the following:

[root@localhost ~]# vim /etc/crypto-policies/config
[root@localhost ~]# update-crypto-policies --is-applied
The configured policy is NOT applied

[jan-cerny]

@yuumasato That's ingenious! @jackmanb1985 Would Yuuma's proposal work for you?

[jackmanb1985]

@jan-cerny I took the approach from an ansible standpoint when crafting up this logic to firstly understand the desired state and if not currently set in --show, then set the value as such in the config file.

As @yuumasato has shown, --is-applied appears to come to the same conclusion and based upon your notes on the test suite above and potentially may allow it to pass. The caveat with both of our examples are if it is not modifying the contents file (which a check is not supposed to), then the test case is always going to fail under these scenarios (because the contents never needs to be updated if ansible identifies it does not need to be changed.)

The test cases sound like they are not fully taking into account ansible best practice under these scenarios?

The alternative would be to inspect the contents of the config file. Again, this would only change the access time as an idempotent condition.

[openshift-ci[bot]]

@jackmanb1985: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-rhcos4-high	f544f5aa83dea89570888724f22557f3d8342d03	link	true	/test e2e-aws-rhcos4-high
ci/prow/e2e-aws-rhcos4-moderate	f544f5aa83dea89570888724f22557f3d8342d03	link	true	/test e2e-aws-rhcos4-moderate
ci/prow/e2e-aws-ocp4-stig	f544f5aa83dea89570888724f22557f3d8342d03	link	true	/test e2e-aws-ocp4-stig

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

[jan-cerny]

Closing because of inactivity.