-
Notifications
You must be signed in to change notification settings - Fork 685
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
Align wireless_disable_interfaces with Ubuntu 22.04 STIG #11886
Conversation
Hi @mpurg. 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 Once the patch is verified, the new status will be reflected by the 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. |
🤖 A k8s content image for this PR is available at: Click here to see how to deploy itIf you alread have Compliance Operator deployed: Otherwise deploy the content and operator together by checking out ComplianceAsCode/compliance-operator and: |
@@ -1,8 +1,6 @@ | |||
# platform = multi_platform_ubuntu | |||
|
|||
if command -v nmcli >/dev/null 2>&1 ; then | |||
nmcli radio all off |
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.
please keep the nmcli, this aligns with CIS
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.
As discussed, we can remove this since CIS 2.0.0 no longer uses nmcli in the remediation.
<ind:textfilecontent54_test check="all" check_existence="none_exist" comment="query /proc/net/wireless" id="test_wireless_disable_interfaces" version="1"> | ||
<ind:object object_ref="object_wireless_disable_interfaces" /> | ||
</ind:textfilecontent54_test> | ||
<ind:textfilecontent54_object id="object_wireless_disable_interfaces" version="1"> |
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.
this check is only identifying if there's any wireless interface, it is not checking if they are disabled.
The shared oval is better
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.
I did this test to check if the interface is still listed:
# cat /proc/net/wireless
Inter-| sta-| Quality | Discarded packets | Missed | WE
face | tus | link level noise | nwid crypt frag retry misc | beacon | 22
wlp2s0: 0000 45. -65. -256 0 0 0 0 1 0
# ip link set wlp2s0 down
# cat /proc/net/wireless
Inter-| sta-| Quality | Discarded packets | Missed | WE
face | tus | link level noise | nwid crypt frag retry misc | beacon | 22
I did find an issue with the regex though.
@@ -93,6 +97,9 @@ ocil: |- | |||
|
|||
If a wireless interface is configured it must be documented and approved by | |||
the local Authorizing Official. | |||
{{% elif 'ubuntu' in product %}} |
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.
this if seems unnecessary as you already has the command introduced above and the note is already in the else below.
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.
If we don't make a special conditional for ubuntu it will show the nmcli
in the else
statement, which is not ideal since we are not relying on nmcli
.
@@ -107,8 +114,26 @@ ocil: |- | |||
{{% endif %}} | |||
|
|||
fixtext: |- | |||
{{% if 'ubuntu' in product %}} |
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.
also mention the nmcli fix
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.
As decided, we are moving away from the nmcli fix.
- Created Ubuntu-specific OVAL to check /proc/net/wireless for enabled interfaces. The existing implementation relies on the interface name starting with "wl", which could be overriden. - Modified remediation to disable the driver modules as suggested by CIS and STIG, instead of disabling wifi using nmcli.
df35984
to
a99b885
Compare
Force pushed a fix to the OVAL regex and rebased to master. @dodys can you re-review? |
Code Climate has analyzed commit a99b885 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 59.4% (0.0% change). View more on Code Climate. |
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.
lgtm, thanks!
Description:
The existing implementation relies on the interface name starting with
"wl", which could be overriden.
instead of disabling wifi using nmcli.