-
Notifications
You must be signed in to change notification settings - Fork 29
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
Replace fips-mode-setup #349
base: master
Are you sure you want to change the base?
Conversation
8611c42
to
3d6b7e7
Compare
src/ipahealthcheck/meta/core.py
Outdated
if not os.path.exists(paths.PROC_FIPS_ENABLED): | ||
fips = "missing {}".format(paths.PROC_FIPS_ENABLED) | ||
logger.debug("Can't find %s, skipping" % | ||
paths.PROC_FIPS_ENABLED) |
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'm a bit torn on this related to other distributions, whether enforcing that the file exist would break them.
When I originally wrote this I didn't want to hold it against a user that they didn't have a tool installed. But the file in /proc should be created by the kernel so if that's missing it points to a larger issue.
I think we should set rval to WARNING in this case. This file is created by the kernel so if FIPS is disabled in the KERNEL that seems odd. A user can suppress the warning if indeed they have this use-case.
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 think we can make the assumption, that this file exists, otherwise, the user is using a custom kernel and is probably not thinking about FIPS at all. Nevertheless, the return value is a warning now.
RHEL10 doesn't support fips-mode-setup, therefore this call has been replaced with simple reading of a proc file. The tests have been edited accordingly, inconsistent has been removed and instead replaced by a test supplying an arbitrary value, that should never occur. Fixes: freeipa#350 Signed-off-by: David Hanina <[email protected]>
Also renamed test_fips_no_fips_enabled to test_fips_no_fips_available as this name is more fitting, meaning the kernel is missing fips. Signed-off-by: David Hanina <[email protected]>
78dfe71
to
1e3225b
Compare
if 'FIPS_MODE_SETUP' not in dir(paths): | ||
paths.FIPS_MODE_SETUP = '/usr/bin/fips-mode-setup' | ||
if 'PROC_FIPS_ENABLED' not in dir(paths): | ||
paths.PROC_FIPS_ENABLED = '/proc/sys/crypto/fips_enabled' |
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.
PROC_FIPS_ENABLED is in ipaplatform/base.py so should be visible in any derived platform. One can override a path but not remove it so I don't think this part is necessary. It's good defensive coding though.
if 'FIPS_MODE_SETUP' not in dir(paths): | ||
paths.FIPS_MODE_SETUP = '/usr/bin/fips-mode-setup' | ||
if 'PROC_FIPS_ENABLED' not in dir(paths): | ||
paths.PROC_FIPS_ENABLED = '/proc/sys/crypto/fips_enabled' |
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.
similar platforms comment.
RHEL10 doesn't support fips-mode-setup, therefore this call has been replaced with simple reading of a proc file. The tests have been edited accordingly, inconsistent has been removed and instead replaced by a test supplying an arbitrary value, that should never occur.
Fixes: #350