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

Make the role FIPS-aware #173

Merged
merged 4 commits into from
Nov 16, 2021
Merged

Make the role FIPS-aware #173

merged 4 commits into from
Nov 16, 2021

Conversation

Jakuje
Copy link
Collaborator

@Jakuje Jakuje commented Nov 9, 2021

Previously, some of our users reported that this role has issues on FIPS systems, because it tries to generate Ed25519 keys, which are not allowed:

linux-system-roles/linux-system-roles.github.io#66

There is a simple workaround to define list of hostkeys excluding this key type such as

sshd_HostKey:
  - /etc/ssh/ssh_host_rsa_key
  - /etc/ssh/ssh_host_ecdsa_key

but the defaults of the role should be FIPS aware.

The complicated thing is the FIPS detection. There are various things that are checked by the applications to turn on the FIPS mode, but generally, the kernel-space FIPS indication in /proc/sys/crypto/fips_enabled is most authoritative and universal place to check across different RHEL versions. Unfortunately, faking this does not work in Github Actions containers so I introduced also a check for /etc/system-fips, which should be also present in RHEL8.

The attached is also a testcase, which is trying to "fake" fips mode with bind mounts and verify the file is not generated. This is applicable only for RHEL7 and RHEL8 as older versions do not have the Ed25519 keys and newer versions already use drop-in directory. Note, that this does not work in the GitHub Actions (therefore the fail_when: false, but it should do the job on real RHEL images.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
ignore failures to bind fips_enabled into /proc/sys/crypto as it looks
like this does not work in the Github Actions containers.

Signed-off-by: Jakub Jelen <jjelen@redhat.com>
@Jakuje
Copy link
Collaborator Author

Jakuje commented Nov 9, 2021

@richm can you have a look if something like this should do the job?

defaults/main.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@richm richm left a comment

Choose a reason for hiding this comment

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

a few comments

The test passes on rhel-6, rhel-7, rhel-8.5, and rhel-9 (with ansible 2.9)

content: 1

- name: Bind mount the file where we need it
mount:
Copy link
Collaborator

Choose a reason for hiding this comment

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

In ansible-core 2.11 and later, this creates a dependency on the ansible.posix.mount module. If it is absolutely necessary to use the mount module, then I guess add a tests/requirements.yml and we'll have to figure out how to include this in our testing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I initially tried to use just mount command, but my ansible was complaining that I should use the module mount so I went on with that. I don't mind rewriting it back if using this module only for testing would be a problem. For now, I will add it to the tests/requirements.yml.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok - I guess we will have to figure out how to incorporate testing requirements in our CI sooner or later . . .

tasks/install.yml Outdated Show resolved Hide resolved
tasks/install.yml Show resolved Hide resolved
@richm
Copy link
Collaborator

richm commented Nov 9, 2021

I also suggest that the github actions should be converted to use the newly released ansible-core 2.12 - but then you may have to figure out how to install the necessary collections where needed . . . or use the ansible 4.x pypi package (and ansible 5.x should be released soon)

@Jakuje
Copy link
Collaborator Author

Jakuje commented Nov 10, 2021

I also suggest that the github actions should be converted to use the newly released ansible-core 2.12 - but then you may have to figure out how to install the necessary collections where needed . . . or use the ansible 4.x pypi package (and ansible 5.x should be released soon)

Thank you for the comments. They should be addressed in the latest commit.

I already fixed the debian-latest action to work in the meantime. For updating the ansible in the actions, I would have to check with their author. But thank you for the suggestion. Right now, it installs from pip. I assume it will be there updated eventually too.

Copy link
Collaborator

@richm richm left a comment

Choose a reason for hiding this comment

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

lgtm

@Jakuje
Copy link
Collaborator Author

Jakuje commented Nov 12, 2021

@mattwillsher can have a look?

@mattwillsher
Copy link
Member

LGTM. Want a release doing?

@Jakuje
Copy link
Collaborator Author

Jakuje commented Nov 12, 2021

LGTM. Want a release doing?

no need to hurry right now, I think. Rich can confirm.

We can keep it open for some time to gather some more feedback.

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.

3 participants