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.

Sign up for GitHub

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Replace fips-mode-setup #349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3a02c7
Replace fips-mode-setup
duzda Feb 17, 2025
872caaf
Mock read_text for acme unknown
duzda Feb 17, 2025
6dbafec
Warn missing fips file
duzda Feb 18, 2025
1e3225b
Remove unnecessary timeout
duzda Feb 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions src/ipahealthcheck/meta/core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import logging
import os
import socket
from pathlib import Path
from ipahealthcheck.core import constants
from ipahealthcheck.core.exceptions import TimeoutError
from ipahealthcheck.core.plugin import Result, duration
Expand All @@ -13,8 +14,8 @@
from ipapython.version import VERSION, API_VERSION
from ipaplatform.paths import paths

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'
Copy link
Collaborator

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.


logger = logging.getLogger()

Expand All @@ -25,31 +26,27 @@ class MetaCheck(Plugin):
def check(self):

rval = constants.SUCCESS
if not os.path.exists(paths.FIPS_MODE_SETUP):
fips = "missing {}".format(paths.FIPS_MODE_SETUP)
logger.debug('%s is not installed, skipping',
paths.FIPS_MODE_SETUP)
if not os.path.exists(paths.PROC_FIPS_ENABLED):
fips = "missing {}".format(paths.PROC_FIPS_ENABLED)
logger.warning("Can't find %s, skipping" %
paths.PROC_FIPS_ENABLED)
rval = constants.WARNING
else:
try:
result = ipautil.run([paths.FIPS_MODE_SETUP,
'--is-enabled'],
capture_output=True,
raiseonerr=False,)
except TimeoutError:
logger.debug('fips-mode-setup timed out')
fips = "check timed out"
rval = constants.ERROR
proc_fips_enable_path = Path(paths.PROC_FIPS_ENABLED)
result_text = proc_fips_enable_path.read_text()
result = int(result_text)
except Exception as e:
logger.debug('fips-mode-setup failed: %s', e)
logger.debug('Reading %s failed: %s' %
(paths.PROC_FIPS_ENABLED, e))
fips = "failed to check"
rval = constants.ERROR
else:
logger.debug(result.raw_output.decode('utf-8'))
if result.returncode == 0:
logger.debug("%s returns %i" %
(paths.PROC_FIPS_ENABLED, result))
if result == 1:
fips = "enabled"
elif result.returncode == 1:
fips = "inconsistent"
elif result.returncode == 2:
elif result == 0:
fips = "disabled"
else:
fips = "unknown"
Expand Down
64 changes: 36 additions & 28 deletions tests/test_meta.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
from ipahealthcheck.core import config, constants
from ipahealthcheck.meta.plugin import registry
from ipahealthcheck.meta.core import MetaCheck
from ipapython import ipautil
from ipaplatform.paths import paths

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'
Copy link
Collaborator

Choose a reason for hiding this comment

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

similar platforms comment.



def gen_result(returncode, output='', error=''):
Expand All @@ -36,7 +35,7 @@ def gen_result(returncode, output='', error=''):

class TestMetaFIPS(BaseTest):
@patch('os.path.exists')
def test_fips_no_fips_mode_setup(self, mock_exists):
def test_fips_no_fips_available(self, mock_exists):
mock_exists.return_value = False

framework = object()
Expand All @@ -48,18 +47,19 @@ def test_fips_no_fips_mode_setup(self, mock_exists):
assert len(self.results) == 1

result = self.results.results[0]
assert result.result == constants.SUCCESS
assert result.result == constants.WARNING
assert result.source == 'ipahealthcheck.meta.core'
assert result.check == 'MetaCheck'
assert result.kw.get('fips') == 'missing %s' % paths.FIPS_MODE_SETUP
assert result.kw.get('fips') == 'missing %s' % paths.PROC_FIPS_ENABLED

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_fips_disabled(self, mock_run, mock_exists):
def test_fips_disabled(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '0'

mock_run.side_effect = [
gen_result(2),
gen_result(0, output='ACME is disabled'),
]

Expand All @@ -78,12 +78,13 @@ def test_fips_disabled(self, mock_run, mock_exists):
assert result.kw.get('fips') == 'disabled'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_fips_enabled(self, mock_run, mock_exists):
def test_fips_enabled(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '1'

mock_run.side_effect = [
gen_result(0),
gen_result(0, output='ACME is disabled'),
]

Expand All @@ -102,12 +103,13 @@ def test_fips_enabled(self, mock_run, mock_exists):
assert result.kw.get('fips') == 'enabled'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_fips_inconsistent(self, mock_run, mock_exists):
def test_fips_unknown(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '2'

mock_run.side_effect = [
gen_result(1),
gen_result(0, output='ACME is disabled'),
]

Expand All @@ -123,15 +125,16 @@ def test_fips_inconsistent(self, mock_run, mock_exists):
assert result.result == constants.SUCCESS
assert result.source == 'ipahealthcheck.meta.core'
assert result.check == 'MetaCheck'
assert result.kw.get('fips') == 'inconsistent'
assert result.kw.get('fips') == 'unknown'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_fips_unknown(self, mock_run, mock_exists):
def test_fips_non_numeric(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = 'test'

mock_run.side_effect = [
gen_result(103),
gen_result(0, output='ACME is disabled'),
]

Expand All @@ -144,20 +147,22 @@ def test_fips_unknown(self, mock_run, mock_exists):
assert len(self.results) == 1

result = self.results.results[0]
assert result.result == constants.SUCCESS
assert result.result == constants.ERROR
assert result.source == 'ipahealthcheck.meta.core'
assert result.check == 'MetaCheck'
assert result.kw.get('fips') == 'unknown'
assert result.kw.get('fips') == 'failed to check'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_fips_failed(self, mock_run, mock_exists):
def test_fips_failed(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True

mock_result.side_effect = [
gen_result(constants.ERROR, output="failed to check"),
]

mock_run.side_effect = [
ipautil.CalledProcessError(
1, 'fips-mode-setup', output='execution failed'
),
gen_result(0, output='ACME is disabled'),
]

Expand Down Expand Up @@ -190,19 +195,20 @@ def test_acme_no_ipa_acme_status(self, mock_exists):
assert len(self.results) == 1

result = self.results.results[0]
assert result.result == constants.SUCCESS
assert result.result == constants.WARNING
assert result.source == 'ipahealthcheck.meta.core'
assert result.check == 'MetaCheck'
assert result.kw.get('acme') == \
'missing %s' % '/usr/sbin/ipa-acme-manage'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_acme_disabled(self, mock_run, mock_exists):
def test_acme_disabled(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '1'

mock_run.side_effect = [
gen_result(0),
gen_result(0, output='ACME is disabled'),
]

Expand All @@ -221,12 +227,13 @@ def test_acme_disabled(self, mock_run, mock_exists):
assert result.kw.get('acme') == 'disabled'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_acme_enabled(self, mock_run, mock_exists):
def test_acme_enabled(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '1'

mock_run.side_effect = [
gen_result(0),
gen_result(0, output='ACME is enabled'),
]

Expand All @@ -245,12 +252,13 @@ def test_acme_enabled(self, mock_run, mock_exists):
assert result.kw.get('acme') == 'enabled'

@patch('os.path.exists')
@patch('pathlib.Path.read_text')
@patch('ipapython.ipautil.run')
def test_acme_unknown(self, mock_run, mock_exists):
def test_acme_unknown(self, mock_run, mock_result, mock_exists):
mock_exists.return_value = True
mock_result.return_value = '1'

mock_run.side_effect = [
gen_result(0),
gen_result(
0,
error="cannot connect to 'https://somewhere/acme/login"
Expand Down