Fix nmcli bug

[anivan-suse]

Description:

• Change to wireless_disable_interfaces remediation to check for nmcli

[openshift-ci]

Hi @anivan-suse. 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_wireless_disable_interfaces' differs.
--- xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
+++ xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
@@ -1,2 +1,8 @@
 
-nmcli radio all off
+if 
+ rpm -q NetworkManager
+then
+ nmcli radio all off
+else
+ echo "NetworkManager package not installed" >&2 
+fi

ansible remediation for rule 'xccdf_org.ssgproject.content_rule_wireless_disable_interfaces' differs.
--- xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
+++ xccdf_org.ssgproject.content_rule_wireless_disable_interfaces
@@ -1,5 +1,6 @@
-- name: Deactivate Wireless Network Interfaces
- command: nmcli radio wifi off
+- name: Gather the package facts
+ package_facts:
+ manager: auto
 tags:
 - CCE-83501-7
 - DISA-STIG-RHEL-08-040110
@@ -17,3 +18,67 @@
 - no_reboot_needed
 - unknown_strategy
 - wireless_disable_interfaces
+
+- name: Check if NetworkManager is installed
+ ansible.builtin.package_facts:
+ manager: auto
+ tags:
+ - CCE-83501-7
+ - DISA-STIG-RHEL-08-040110
+ - NIST-800-171-3.1.16
+ - NIST-800-53-AC-18(3)
+ - NIST-800-53-AC-18(a)
+ - NIST-800-53-CM-6(a)
+ - NIST-800-53-CM-7(a)
+ - NIST-800-53-CM-7(b)
+ - NIST-800-53-MP-7
+ - PCI-DSS-Req-1.3.3
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - unknown_strategy
+ - wireless_disable_interfaces
+
+- name: Error message when NetworkManager not installed
+ fail:
+ msg: NetworkManager package not installed
+ when: '''NetworkManager'' not in ansible_facts.packages'
+ tags:
+ - CCE-83501-7
+ - DISA-STIG-RHEL-08-040110
+ - NIST-800-171-3.1.16
+ - NIST-800-53-AC-18(3)
+ - NIST-800-53-AC-18(a)
+ - NIST-800-53-CM-6(a)
+ - NIST-800-53-CM-7(a)
+ - NIST-800-53-CM-7(b)
+ - NIST-800-53-MP-7
+ - PCI-DSS-Req-1.3.3
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - unknown_strategy
+ - wireless_disable_interfaces
+
+- name: Deactivate Wireless Network Interfaces
+ command: nmcli radio wifi off
+ when: '''NetworkManager'' in ansible_facts.packages'
+ tags:
+ - CCE-83501-7
+ - DISA-STIG-RHEL-08-040110
+ - NIST-800-171-3.1.16
+ - NIST-800-53-AC-18(3)
+ - NIST-800-53-AC-18(a)
+ - NIST-800-53-CM-6(a)
+ - NIST-800-53-CM-7(a)
+ - NIST-800-53-CM-7(b)
+ - NIST-800-53-MP-7
+ - PCI-DSS-Req-1.3.3
+ - low_complexity
+ - medium_disruption
+ - medium_severity
+ - no_reboot_needed
+ - unknown_strategy
+ - wireless_disable_interfaces

[marcusburghardt]

The rule description explicitly mention nmcli command and the remediations are based on it. I wonder if would it be better to ensure the NetworkManager package (for applicable distros) is installed instead of alerting the user and stopping the remediation. In this case, it sounds reasonable to me to ensure the package installation in the remediation. Any thoughts?

[yuumasato]

The rule description explicitly mention nmcli command and the remediations are based on it. I wonder if would it be better to ensure the NetworkManager package (for applicable distros) is installed instead of alerting the user and stopping the remediation. In this case, it sounds reasonable to me to ensure the package installation in the remediation. Any thoughts?

That is a good question, and I'll make more questions to try to answer it.
What is the reason that NetworkManager is not installed?
Wouldn't installing it just to disable a NIC increase the surface attack of the system?
Are there other means to disable the wi-fi interface without nmcli?

If NetworkManager needs to be installed I'd prefer to have a rule specific for that, so it is more explicit.
Also, one could argue this rule should have a CPE platform for NetworkManager package, but I don't feel strongly about it.

[marcusburghardt]

The rule description explicitly mention nmcli command and the remediations are based on it. I wonder if would it be better to ensure the NetworkManager package (for applicable distros) is installed instead of alerting the user and stopping the remediation. In this case, it sounds reasonable to me to ensure the package installation in the remediation. Any thoughts?

That is a good question, and I'll make more questions to try to answer it. What is the reason that NetworkManager is not installed? Wouldn't installing it just to disable a NIC increase the surface attack of the system? Are there other means to disable the wi-fi interface without nmcli?

If NetworkManager needs to be installed I'd prefer to have a rule specific for that, so it is more explicit. Also, one could argue this rule should have a CPE platform for NetworkManager package, but I don't feel strongly about it.

I remember to work on this rule some months ago and I deeply researched about alternative ways to disable wireless interfaces. It is possible by manipulating files, but the logic would be "extremely" complex to precisely find the patterns and make the solution stable. My conclusion is that nmcli is the best option for this context.

I just remembered the requires attribute for the rules. So, in my opinion, this would be the ideal scenario:

• Create a new rule for installing NetworkManager package
  • We already have the package_installed template and this would help.
• Set the requires attribute in the wireless_disable_interfaces rule, indicating the new rule which installs the NetworkManager package.

What do you think @anivan-suse and @yuumasato ?

[anivan-suse]

The rule description explicitly mention nmcli command and the remediations are based on it. I wonder if would it be better to ensure the NetworkManager package (for applicable distros) is installed instead of alerting the user and stopping the remediation. In this case, it sounds reasonable to me to ensure the package installation in the remediation. Any thoughts?

@marcusburghardt @yuumasato

Hey, so I'm pretty confident (not entirely) that we cannot require NetworkManager. Suse supports two networking set-up tools: wicked and networkmanager (which includes nmcli). Wicked is used it graphic-less servers while NetworkManager (nmcli) is used for systems with graphics displays. This is why we went the route of just checking for existence of NetworkManager rather than requiring it.

[marcusburghardt]

The rule description explicitly mention nmcli command and the remediations are based on it. I wonder if would it be better to ensure the NetworkManager package (for applicable distros) is installed instead of alerting the user and stopping the remediation. In this case, it sounds reasonable to me to ensure the package installation in the remediation. Any thoughts?

@marcusburghardt @yuumasato

Hey, so I'm pretty confident (not entirely) that we cannot require NetworkManager. Suse supports two networking set-up tools: wicked and networkmanager (which includes nmcli). Wicked is used it graphic-less servers while NetworkManager (nmcli) is used for systems with graphics displays. This is why we went the route of just checking for existence of NetworkManager rather than requiring it.

Thanks for the information @anivan-suse. It is a fair point. In that case I would be fine to only warning the user in this rule.

[yuumasato]

The Automatus failures are fine, the rule doesn't have tests and cannot actually be tested in containers.

[codeclimate]

Code Climate has analyzed commit 569b9f6 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 46.8% (0.0% change).

View more on Code Climate.