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

Improve CIS UDF kernel module check #3562

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

stmcginnis
Copy link
Contributor

Issue number:

N/A

Description of changes:

The level 2 check 1.1.1.1 verifies mounting UDF filesystems is disabled. The current check for whether it is already loaded was not correct. Luckily there is a second check as part of this control to make sure loading is disabled. If the setting for loading is to not allow it, but the module is already loaded, the check does not return the expected output. So it would still report failure, but it is less than ideal for reporting the actual issue.

This changes the check for whether the module is loaded to correctly identify if the module is loaded or not before checking whether the ability has been disabled.

Testing done:

Ran the level 2 report on a default deployment and verified it reported FAIL for this check.

Checked the loaded but disabled scenario:

bash-5.1# lsmod | grep udf

bash-5.1# modprobe -v -n udf
insmod /lib/modules/5.15.134/kernel/drivers/cdrom/cdrom.ko.xz 
insmod /lib/modules/5.15.134/kernel/lib/crc-itu-t.ko.xz 
insmod /lib/modules/5.15.134/kernel/fs/udf/udf.ko.xz

bash-5.1# insmod /lib/modules/5.15.134/kernel/drivers/cdrom/cdrom.ko.xz

bash-5.1# insmod /lib/modules/5.15.134/kernel/lib/crc-itu-t.ko.xz

bash-5.1# insmod /lib/modules/5.15.134/kernel/fs/udf/udf.ko.xz

bash-5.1# grep udf /proc/modules 
udf 139264 0 - Live 0xffffffffc0636000
crc_itu_t 16384 1 udf, Live 0xffffffffc0631000
cdrom 77824 1 udf, Live 0xffffffffc061d000

bash-5.1# apiclient set kernel.modules.udf.allowed=false

bash-5.1# modprobe -v -n udf                             

bash-5.1# apiclient report cis -l 2
...
[FAIL] 1.1.1.1   Ensure mounting of udf filesystems is disabled (Automatic)
...

Then unloaded the module and made sure it correctly identifies the module is not loaded and loading is disabled:

bash-5.1# rmmod udf

bash-5.1# modprobe -v -n udf                             
install /bin/true 

bash-5.1# apiclient report cis -l 2
...
[PASS] 1.1.1.1   Ensure mounting of udf filesystems is disabled (Automatic)
...

Then unload

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

Copy link
Contributor

@bcressey bcressey left a comment

Choose a reason for hiding this comment

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

Nice!

sources/bloodhound/src/lib.rs Show resolved Hide resolved
The level 2 check 1.1.1.1 verifies mounting UDF filesystems is disabled.
The current check for whether it is already loaded was not correct.
Luckily there is a second check as part of this control to make sure
loading is disabled. If the setting for loading is to not allow it, but
the module is already loaded, the check does not return the expected
output. So it would still report failure, but it is less than ideal for
reporting the actual issue.

This changes the check for whether the module is loaded to correctly
identify if the module is loaded or not before checking whether the
ability has been disabled.

Signed-off-by: Sean McGinnis <stmcg@amazon.com>
Copy link
Contributor

@yeazelm yeazelm left a comment

Choose a reason for hiding this comment

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

LGTM!

@stmcginnis stmcginnis merged commit 8220063 into bottlerocket-os:develop Nov 3, 2023
1 of 46 checks passed
@stmcginnis stmcginnis deleted the cis-udf branch November 3, 2023 15:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants