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

Add FIPS prepare feature enabling FIPS mode on RHEL and CentOS #3344

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

The-Mule
Copy link
Contributor

@The-Mule The-Mule commented Nov 6, 2024

This adds prepare feature for enabling FIPS mode. Notice that disabling FIPS mode is unsupported and hence not implemented. Moreover, keep in mind that the proper way of enabling FIPS mode is to install the system in FIPS mode. The procedure depicted in the Ansible playbook is the only way of enabling it after installation. Since the procedure requires modification of kernel command line, it only supports Guests that allows that (i.e. not containers). Last but not least, the procedure is only valid for CentOS and RHEL (Fedora FIPS mode is an experimental possibility that has no support at all).

This PR is based on #2773 since I am not allowed to continue the work there.

Relates to #1062

Pull Request Checklist

  • implement the feature
  • write the documentation
  • extend the test coverage
  • update the specification
  • adjust plugin docstring
  • modify the json schema
  • mention the version
  • include a release note

Signed-off-by: Ondrej Moris <[email protected]>
@The-Mule
Copy link
Contributor Author

The-Mule commented Nov 6, 2024

FYI: Is there a way to inhibit feature on distributions other than RHEL and CentOS in feature.py?

@happz
Copy link
Collaborator

happz commented Nov 6, 2024

FYI: Is there a way to inhibit feature on distributions other than RHEL and CentOS in feature.py?

Via ansible_distribution == "RedHat" checks in when.

https://github.com/teemtee/tmt/blob/main/tmt/steps/prepare/feature/epel-enable.yaml#L23


def is_unsupported(self) -> bool:
return isinstance(self.guest, (tmt.steps.provision.local.GuestLocal,
tmt.steps.provision.podman.GuestContainer))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Class enumeration will fail as soon as someone comes up with their own plugin that would not support this operation. I'd recommend adding something like localhost attribute to Guest class, see https://github.com/teemtee/tmt/blob/main/tmt/steps/provision/local.py#L21 - it's set to False by default, localhost plugins sets it to True, and code checks this flag - not the class - when it's deciding about localhost-ish features. So I guess we need something like fips_support or something similar, set it to True by default & False in these two classes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me think about it. Adding a new flag might not be needed in the end since there is basically an equivalence between fips_support and "is_not_container" and I can most likely detect that in the playbook as well by inspecting init process control groups. That might be more tmt-friendly solution.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just to be sure, will such autodetection work with Image mode, a container-ish runtime?

We might add the is_container or similar flag to Guest classes if it turns out it's needed for more than just FIPS. I'd like all involved plugins to see the same picture, i.e. one plugin detecting a fact one way while another one would use completely different steps to discover the same answer would be something I'd wish to address. Not a blocker for now though, just a thought.

@The-Mule
Copy link
Contributor Author

The-Mule commented Nov 6, 2024

FYI: Is there a way to inhibit feature on distributions other than RHEL and CentOS in feature.py?

Via ansible_distribution == "RedHat" checks in when.

https://github.com/teemtee/tmt/blob/main/tmt/steps/prepare/feature/epel-enable.yaml#L23

Right, I understand that. I would prefer to abort sooner that in the playbook (throwing unsupported exception) but now thinking about that that I guess I will add a task to the playbook that will fail on other distros.

@happz
Copy link
Collaborator

happz commented Nov 6, 2024

FYI: Is there a way to inhibit feature on distributions other than RHEL and CentOS in feature.py?

Via ansible_distribution == "RedHat" checks in when.
https://github.com/teemtee/tmt/blob/main/tmt/steps/prepare/feature/epel-enable.yaml#L23

Right, I understand that. I would prefer to abort sooner that in the playbook (throwing unsupported exception)

That would require tmt parsing somehow facts like distro, /etc/os-release and so on, and while I'm not sure about others, I for one would prefer to avoid that kind of code in tmt. It stinks with regular expressions, exceptions and corner cases. If Ansible can do the work, I'm fine with using it.

but now thinking about that that I guess I will add a task to the playbook that will fail on other distros.

Out of curiosity: does it need to fail? I think we shouldn't dive into deep waters, but some day in the future, is FIPS applicable to other distros?

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.

2 participants