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

A new rule system_boot_in_fips_mode #12671

Merged
merged 7 commits into from
Dec 16, 2024

Conversation

Mab879
Copy link
Member

@Mab879 Mab879 commented Dec 4, 2024

Description:

Add new rule system_boot_in_fips_mode.

Rationale:

Since fips-mode-setup is gone is RHEL 10 we need to adjust our content for it.

@Mab879 Mab879 added this to the 0.1.76 milestone Dec 4, 2024
Copy link

github-actions bot commented Dec 4, 2024

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

ol8 (from CTF) Environment (using Fedora as testing environment)
Open in Gitpod

Fedora Testing Environment
Open in Gitpod

Oracle Linux 8 Environment
Open in Gitpod

@Mab879 Mab879 marked this pull request as draft December 4, 2024 20:57
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 4, 2024
@Mab879 Mab879 changed the title Move to is_fips_mode_enabled for RHEL 10 A new rule system_boot_in_fips_mode Dec 4, 2024
@Mab879 Mab879 added the New Rule Issues or pull requests related to new Rules. label Dec 4, 2024
@jan-cerny
Copy link
Collaborator

This new rule will check that the system currently runs in FIPS mode. But, How will we verify that the system is configured to start in FIPS mode?

@Mab879
Copy link
Member Author

Mab879 commented Dec 5, 2024

This new rule will check that the system currently runs in FIPS mode. But, How will we verify that the system is configured to start in FIPS mode?

I have added the grub2 check as well.

title: 'Verify that the system was booted with fips=1'

description: |-
On a system where FIPS 14032 mode is enabled, the system must be booted with the
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
On a system where FIPS 14032 mode is enabled, the system must be booted with the
On a system where FIPS 140-2 mode is enabled, the system must be booted with the
Suggested change
On a system where FIPS 14032 mode is enabled, the system must be booted with the
On a system where FIPS 140-3 mode is enabled, the system must be booted with the

Which one is the correct one?

Copy link
Member Author

Choose a reason for hiding this comment

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

140-3

Choose a reason for hiding this comment

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

In crypto's docs, we prefer to use "FIPS 140" now, since there may be a FIPS 140-4 a lot sooner than the time it took to go from -2 to -3.

@Mab879 Mab879 marked this pull request as ready for review December 10, 2024 13:24
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Used by openshift-ci bot. label Dec 10, 2024
@@ -1,7 +1,7 @@
documentation_complete: true


title: Verify '/proc/sys/crypto/fips_enabled' exists
title: Verify '/proc/sys/crypto/fips_enabled' exists

Choose a reason for hiding this comment

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

The mere existence of /proc/sys/crypto/fips_enabled is not sufficient. All kernels that support enabling FIPS mode will expose this file in user space. It must contain 1 for FIPS mode to be actually enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have removed this rule from RHEL 10.


<ind:textfilecontent54_object id="obj_{{{ rule_id }}}_mode_exists" version="1">
<ind:filepath>/proc/cmdline</ind:filepath>
<ind:pattern operation="pattern match">.+fips*=1.+</ind:pattern>

Choose a reason for hiding this comment

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

Why does the pattern have a * between fips and the equals sign?

If this is a regex (which the starting .+ seems to indicate), then this would match fipsssss=1 or fip=1, which don't enable FIPS mode.

Also, this would match notreallyfips=1 due to the .+ before; That regex should probably instead be anchored by matching a word boundary, or explicitly search for whitespace before and after fips=1.

However, there's really no difference between the kernel command line containing fips=1 and /proc/sys/crypto/fips_enabled containing 1, so I'd propose checking for the latter instead, because it's much easier to do.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the tip. That makes the check much easier.

title: 'Verify that the system was booted with fips=1'

description: |-
On a system where FIPS 14032 mode is enabled, the system must be booted with the

Choose a reason for hiding this comment

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

In crypto's docs, we prefer to use "FIPS 140" now, since there may be a FIPS 140-4 a lot sooner than the time it took to go from -2 to -3.

@jan-cerny jan-cerny self-assigned this Dec 13, 2024
Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

The rule description isn't consistent with the OVAL check. The rule description says that users should check /proc/cmdline, but the OVAL check reads /proc/sys/crypto/fips_enabled. Although both ways of checking are effectively equivalent, you should keep the OVAL check aligned with the rule description or explain this difference in the rule description.

@Mab879
Copy link
Member Author

Mab879 commented Dec 13, 2024

The rule description isn't consistent with the OVAL check. The rule description says that users should check /proc/cmdline, but the OVAL check reads /proc/sys/crypto/fips_enabled. Although both ways of checking are effectively equivalent, you should keep the OVAL check aligned with the rule description or explain this difference in the rule description.

I have adjust the rule description.

Copy link

codeclimate bot commented Dec 13, 2024

Code Climate has analyzed commit e4afdf5 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 60.9% (0.0% change).

View more on Code Climate.

@Mab879
Copy link
Member Author

Mab879 commented Dec 13, 2024

/packit retest-failed

Copy link
Collaborator

@jan-cerny jan-cerny left a comment

Choose a reason for hiding this comment

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

I have built RHEL 10 content and verified that the rule is present in the built data stream.

@jan-cerny jan-cerny merged commit 301d803 into ComplianceAsCode:master Dec 16, 2024
99 of 105 checks passed
@Mab879 Mab879 deleted the fix_fips_rhel10 branch December 16, 2024 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Rule Issues or pull requests related to new Rules.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants