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

security: call /usr/libexec/fips-setup-helper #5824

Merged
merged 1 commit into from
Sep 5, 2024

Conversation

t184256
Copy link
Contributor

@t184256 t184256 commented Aug 13, 2024

crypto-policies now ships a helper for anaconda to call
in order to just "do the right thing"
and make it not anaconda's responsibility.

Copy link

@neverpanic neverpanic left a comment

Choose a reason for hiding this comment

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

Are there any version requirements associated with this, e.g., >= c10s?

@t184256
Copy link
Contributor Author

t184256 commented Aug 13, 2024

For this PR I had f41 rawhide in mind, and the crypto-policies version to ship the helper is crypto-policies-20240807-1.git5795660.fc41.

For c10s, that'd be crypto-policies-20240807-1.git7ea320f.el10

ELN is special in that crypto-policies ELN follows c10s and not rawhide, and thus the first ELN version to ship the helper is crypto-policies-20240807-1.git7ea320f.eln141

I think it makes sense to file a c10s PR after this one gets merged.

@jkonecny12
Copy link
Member

Hi @t184256 thanks a lot for this PR.
I wonder, is this still required https://github.com/rhinstaller/anaconda/blob/master/pyanaconda/modules/security/installation.py#L39 ?

@jkonecny12
Copy link
Member

I think it makes sense to file a c10s PR after this one gets merged.

We also need a RHEL issue filed.

@t184256
Copy link
Contributor Author

t184256 commented Aug 15, 2024

Hi @t184256 thanks a lot for this PR. I wonder, is this still required https://github.com/rhinstaller/anaconda/blob/master/pyanaconda/modules/security/installation.py#L39 ?

I'd say yes. It was outright load-bearing in FIPS 140-2 times, and while it became less important since FIPS 140-3, some of the restrictions are not really enforced, but communicated through explicit indicators. In my book, having the configs in place before the transaction begins is better than applying them at a random mid-transaction moment when the crypto-policies scriptlet gets triggered. The actions that PreconfigureFIPSTask performs are up to date.

@t184256
Copy link
Contributor Author

t184256 commented Aug 19, 2024

RHEL-9 backport is crypto-policies-20240815-1.gite217f03.el9

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

@t184256 Please rebase the PR to latest master and repush for the tests to run again.

@KKoukiou
Copy link
Contributor

/kickstart-tests --testtype smoke

@KKoukiou
Copy link
Contributor

Hi @t184256 thanks a lot for this PR. I wonder, is this still required https://github.com/rhinstaller/anaconda/blob/master/pyanaconda/modules/security/installation.py#L39 ?

This change just wraps the existing command in a script. There is not way this can be affected by this PR.

@KKoukiou
Copy link
Contributor

/build-image

Copy link

Images built based on commit ee0d6aa:

  • boot.iso: success

Download the images from the bottom of the job status page.

@KKoukiou
Copy link
Contributor

KKoukiou commented Sep 4, 2024

Hi @t184256 - have your tested that interactively?

@t184256
Copy link
Contributor Author

t184256 commented Sep 4, 2024

Hm, tried it now with reanaconda against f41 and ran into a problem:

Some packages, groups or modules are missing.

Problems in request:
missing packages: /usr/libexec/fips-setup-helper

At the same time, dnf install /usr/libexec/fips-setup-helper works fine for me on an different, already installed f41: Package "crypto-policies-20240826-1.gite824389.fc41.noarch" is already installed.

I'm actually not sure why does that happen.

EDIT: same with rawhide and a rebased version of this PR

@KKoukiou
Copy link
Contributor

KKoukiou commented Sep 4, 2024

/build-image

Copy link

github-actions bot commented Sep 4, 2024

Images built based on commit ee0d6aa:

  • boot.iso: success

Download the images from the bottom of the job status page.

@KKoukiou
Copy link
Contributor

KKoukiou commented Sep 5, 2024

@t184256 maybe you can use the boot.iso image generated by the CI containing your changes for testing? #5824 (comment)

I am not familiar with fips - therefore not sure what I should be testing here.

@t184256
Copy link
Contributor Author

t184256 commented Sep 5, 2024

Will try when it finishes downloading one eternity later =)

To install in FIPS mode, one should add fips=1 option to the kernel cmdline when booting into anaconda. Then just install as usual.

For my testing above, I've used reanaconda as follows:

./reanaconda.py prime --sensible --tree https://dl.fedoraproject.org/pub/fedora/linux/development/rawhide/Everything/x86_64/os --append 'fips=1' -m 3G
pushd ~/code/anaconda  # with my PR checked out
scripts/makeupdates
popd
./reanaconda.py updates ~/code/anaconda/updates.img

and next-next-finished to a gui popup with the error.

@t184256
Copy link
Contributor Author

t184256 commented Sep 5, 2024

Same error =/

I don't have any better ideas than use the package name instead of the script name; despite it was previously a script name and it worked just fine.

crypto-policies now ships a helper for anaconda to call
in order to just "do the right thing"
and make it not anaconda's responsibility.
@t184256
Copy link
Contributor Author

t184256 commented Sep 5, 2024

Pushed a change that defines dependency as package name. This worked for me, i.e., there's no error and the helper is being invoked.

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

@t184256 so it's now tested and it works with the new image?

@t184256
Copy link
Contributor Author

t184256 commented Sep 5, 2024

yes, the 123d819 version booted with `fips=1':

  1. does not error out when installing dependencies
  2. installs crypto-policies-scripts
  3. invokes the helper at the end of the installation

@KKoukiou
Copy link
Contributor

KKoukiou commented Sep 5, 2024

/kickstart-tests --testtype smoke

@jkonecny12
Copy link
Member

Hi @t184256 should we backport this to RHEL-9 and RHEL-10? Do we have already filed issues for these?

@jkonecny12 jkonecny12 added f42 Fedora 42 and removed port to RHEL10 f41 labels Sep 5, 2024
@t184256
Copy link
Contributor Author

t184256 commented Sep 5, 2024

@jkonecny12

RHEL-10: yes, that one is of my direct interest. I've filed https://issues.redhat.com/browse/RHEL-57680

RHEL-9: we don't actually plan to change what switching into FIPS mode means on RHEL-9. On the other hand, I feet like there's value in separation of concerns for the anaconda side of things as well, so I've backported the helper to 9, and I'd like to leave the decision of using or not using it up to you.

@jkonecny12
Copy link
Member

@jstodola what do you think about RHEL-9 backport?

@jstodola
Copy link
Contributor

jstodola commented Sep 5, 2024

I'm fine with porting the change to RHEL-9.

@KKoukiou KKoukiou merged commit 443d5e6 into rhinstaller:master Sep 5, 2024
24 of 25 checks passed
@jkonecny12 jkonecny12 added the port to RHEL9 The pull request needs to be ported to RHEL 9. label Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
f42 Fedora 42 port to RHEL9 The pull request needs to be ported to RHEL 9. port to RHEL10
Development

Successfully merging this pull request may close these issues.

5 participants