Skip to content

OCPBUGS-65684: Fix invalid random source in FIPS 140-only mode in FIPS mode#2159

Merged
tlbueno merged 1 commit intocoreos:mainfrom
tlbueno:fips_fork
Nov 18, 2025
Merged

OCPBUGS-65684: Fix invalid random source in FIPS 140-only mode in FIPS mode#2159
tlbueno merged 1 commit intocoreos:mainfrom
tlbueno:fips_fork

Conversation

@tlbueno
Copy link
Copy Markdown
Member

@tlbueno tlbueno commented Nov 18, 2025

When igntion is compiled with GOEXPERIMENT=strictfipsruntime and running in a computer with FIPS enabled, the random source is invalid.

Instead of use a custom random on TLS config, we must do not set a random source at all as it will use crypto/rand.Reader by default

@tlbueno tlbueno self-assigned this Nov 18, 2025
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses an issue with using an invalid random source in FIPS 140-only mode. By removing the custom random source from the TLS configuration, the code now relies on the FIPS-compliant default (crypto/rand.Reader), which resolves the reported error. The change is clear and the added comment provides good context. I've added one suggestion for a follow-up refactoring to improve code clarity now that a potential source of errors has been removed.

@dustymabe
Copy link
Copy Markdown
Member

This passes a local fips test for me, but I don't understand the implications of not setting the rand value in non-fips mode.

@tlbueno tlbueno changed the title Fix invalid random source in FIPS 140-only mode in FIPS mode OCPBUGS-65684: Fix invalid random source in FIPS 140-only mode in FIPS mode Nov 18, 2025
@prestist
Copy link
Copy Markdown
Collaborator

prestist commented Nov 18, 2025

@dustymabe, yeah I do not either, we could build a patch file, and simply apply it in packaging for fips-only targets?

We did something like this for adding rdcore for the longest time.

@dustymabe
Copy link
Copy Markdown
Member

looks like we need to run gofmt.

@tlbueno tlbueno force-pushed the fips_fork branch 4 times, most recently from d23815e to 8b8889a Compare November 18, 2025 17:02
@prestist prestist force-pushed the fips_fork branch 2 times, most recently from 1700022 to e1a9fa8 Compare November 18, 2025 17:25
@tlbueno tlbueno force-pushed the fips_fork branch 2 times, most recently from 860a137 to 18d5da5 Compare November 18, 2025 18:14
Copy link
Copy Markdown
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

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

LGTM

…S mode

When igntion is compiled with GOEXPERIMENT=strictfipsruntime and
running in a computer with FIPS enabled, the random source is invalid.

When FIPS is enabled, instead of use a custom random on TLS config,
do not set a random source at all as it will use crypto/rand.Reader by
default

Co-authored-by: Steven Presti <47181335+prestist@users.noreply.github.com>
Co-authored-by: Dusty Mabe <dusty@dustymabe.com>

Signed-off-by: Tiago Bueno <tiago.bueno@gmail.com>
@tlbueno tlbueno merged commit 372ffc2 into coreos:main Nov 18, 2025
10 checks passed
@travier
Copy link
Copy Markdown
Member

travier commented Nov 19, 2025

This passes a local fips test for me, but I don't understand the implications of not setting the rand value in non-fips mode.

Not reading directly from urandom means that Ignition might block on reading random bytes until the random pool is initialized. But on FIPS mode, the random algorithm are slightly different, so I'm not even sure it will block at all.

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.

4 participants