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

CORE-3167 relax fips enabled behavior #18766

Conversation

michael-redpanda
Copy link
Contributor

@michael-redpanda michael-redpanda commented Jun 4, 2024

This PR changes the behavior of the fips_mode node config flag:

  • Converts from bool to an enum
  • Adds three new values:
    • disabled - FIPS mode disabled
    • enabled - FIPS mode enabled and /proc/sys/crypto/fips_enabled must equal 1 or Redpanda will exit
    • permissive - FIPS mode enabled and /proc/sys/crypto/fips_enabled is checked for 1 but if not 1, Redpanda will log a warning

Backports Required

  • none - not a bug fix
  • none - this is a backport
  • none - issue does not exist in previous branches
  • none - papercut/not impactful enough to backport
  • v24.1.x
  • v23.3.x
  • v23.2.x

Release Notes

  • None

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda michael-redpanda requested a review from a team as a code owner June 4, 2024 11:02
@michael-redpanda michael-redpanda self-assigned this Jun 4, 2024
@michael-redpanda michael-redpanda requested review from a team, andijcr and graphcareful and removed request for a team June 4, 2024 11:02
@Deflaimun
Copy link
Contributor

Deflaimun commented Jun 4, 2024

Just out of curiosity. Why not use 0; 1 ; 2 as the values, as opposed to the strings enabled; disabled; permissive ?

@michael-redpanda
Copy link
Contributor Author

Just out of curiosity. Why not use 0; 1 ; 2 as the values, as opposed to the strings enabled; disabled; permissive ?

Because a user will be the one setting the fips_mode configuration value in redpanda.yaml. I think using the strings is more expressive than using numbers

micheleRP
micheleRP previously approved these changes Jun 4, 2024
Copy link

@micheleRP micheleRP left a comment

Choose a reason for hiding this comment

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

please see minor doc suggestions!

graphcareful
graphcareful previously approved these changes Jun 4, 2024
src/v/config/types.h Show resolved Hide resolved
src/v/config/convert.h Outdated Show resolved Hide resolved
andijcr
andijcr previously approved these changes Jun 4, 2024
src/v/config/convert.h Outdated Show resolved Hide resolved
Signed-off-by: Michael Boquard <michael@redpanda.com>
Signed-off-by: Michael Boquard <michael@redpanda.com>
This ensures that a test that wishes to override the
FIPS mode flag doesn't get that overwritten later.

Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda
Copy link
Contributor Author

Force push ea62122:

  • Updates to docs
  • Fixed some weirdness in convert
  • Added tests

})

@cluster(num_nodes=3)
def test_startup(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe parameterize by mode, adding testing capability to ensure it crashes when expected in enabled mode as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When RP is not installed to /opt/redpanda, some of the config
files will point to non-existant files.  This change will make
it so one can run Redpanda in FIPS mode in DT.

Signed-off-by: Michael Boquard <michael@redpanda.com>
Signed-off-by: Michael Boquard <michael@redpanda.com>
@michael-redpanda michael-redpanda force-pushed the CORE-3167-Relax-fips-enabled branch from ea62122 to 82b4733 Compare June 4, 2024 18:41
@michael-redpanda
Copy link
Contributor Author

Force push 82b4733:

  • Added missing openssl.conf template file

@michael-redpanda michael-redpanda merged commit 086dc22 into redpanda-data:dev Jun 5, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants