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

Backport fips-crypto-policies module #35

Closed

Conversation

neverpanic
Copy link
Contributor

Users that enable FIPS mode using fips-mode-seutp --enable and later disable it using fips-mode-setup --disable commonly notice that fips-mode-setup --check tells them that their system is in an inconsistent state. This is because (among other things) fips-mode-setup --disable does not undo adding the fips module to the initramfs out of an abundance of caution to avoid breaking a system's boot.

This situation is not great, because users are often confused by the fips-mode-setup --check message and will not stop using fips-mode-setup --disable (despite the manpage clearly saying that it's for testing purposes only and unsupported).

We plan on solving this issue once and for all in CentOS 10 Stream by eliminating the possibility of inconsistency. For this to work, we need a single source of truth that decides whether FIPS is enabled or not, and all other parts need to follow along automatically. The obvious choice for the single source of truth is the kernel command line, i.e. if it contains fips=1, FIPS should be enabled, if it doesn't or it contains fips=0, FIPS mode should be disabled.

This backport from upstream achieves a significant part of that, by automatically switching the crypto-policy to FIPS on systems booted with fips=1 on the kernel command line. The crypto-policies package has been adjusted to detect and transparently deal with situation (especially during package updates) in F41 and rawhide with upstream commit e7b94d2a309c5348aa534c5a1d9718261a2ec89a.

Cherry picked from commits bd3c1e1cc2f656f7ee4ff47e00ca716d52a86a3d and a2096dafdbfc88eed91ce34b1f4d27e7eb7ca839.

Related: CRYPTO-13556

pvalena
pvalena previously approved these changes Sep 13, 2024
Copy link
Contributor

@pvalena pvalena left a comment

Choose a reason for hiding this comment

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

LGTM.

@pvalena
Copy link
Contributor

pvalena commented Sep 13, 2024

Let me sync dist-git first, and then I'll merge&build.

@neverpanic
Copy link
Contributor Author

FWIW, I can reproduce the OpenSuSE test failures locally, and they seem to be caused by /dev/disk not existing (for whatever reason). The same failures are also present on main, so this doesn't seem to be a regression introduced by my PR.

@pvalena
Copy link
Contributor

pvalena commented Sep 13, 2024

Sure, np, I've just not yet removed the tests. I plan to focus solely on Fedora here.

@neverpanic
Copy link
Contributor Author

I've rebased on commit that should re-establish the sync with dist-git.

@pvalena
Copy link
Contributor

pvalena commented Sep 16, 2024

Hi, thanks for working on this -- I just realized there's 103 rebase pending for a long time, and I'll put this on top of it (just in case there're some merge conflicts). Will do the rebase fully (and resync) manually, as AFAIK currently it fedpkg import (I plan to implement support directly in source-git in the future).

@neverpanic
Copy link
Contributor Author

Sure. Just let me know if I can help by rebasing this PR, or doing anything else.

neverpanic and others added 3 commits September 19, 2024 13:35
For a system that uses crypto-policies to be switched to FIPS mode
correctly, it needs to be

- booted with `fips=1` on the kernel command line
- switched to the FIPS crypto-policy (or a policy derived from it)
- have the fips dracut module enabled

On older systems, there were additional steps, for example, creating
`/etc/system-fips`.

We have repeatedly seen inconsistencies between those different toggles,
either because the user space tooling to switch between those does not
(for reliability, maintainability, and compliance reasons) undo some of
the steps it does when disabling FIPS mode, or because other
installation methods (bootc, containers, image builder) independently do
some of those steps. Eventually, all of these ended with user confusion.

We can avoid this situation by eliminating the difference by treating
the `fips=1` kernel command line switch as a single source of truth, and
making all others follow automatically. This module provides this for
crypto-policies, by adding bind-mounts before pivot if the system has
not already been switched to a FIPS-based crypto-policy.

This requires some support from the crypto-policies package (because it
needs to deal with the bind mounts when a user calls
`update-crypto-policies --set`), so make it a no-op unless

 - `fips=1` is on the kernel command line
 - crypto-policies is installed
 - crypto-policies supports the bind-mounts (indicated by the presence
   of the `default-fips-config` file)
 - the policy isn't already FIPS

These checks should make this safe to add to the initramfs on all
current systems.

The bind-mounts also need to happen in the initramfs already, because
systemd links against OpenSSL, and doing them later means that systemd
will start with an OpenSSL configuration that isn't tailored for FIPS.

See also [1], which adds the user space support to crypto-policies,
along with a systemd service that does the same steps in case dracut
hasn't already done them (which is useful for environments that don't
use an initramfs like containers).

  [1]: https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/merge_requests/191

Signed-off-by: Clemens Lang <[email protected]>
(cherry picked from commit bd3c1e1cc2f656f7ee4ff47e00ca716d52a86a3d)
(cherry picked from commit a2096dafdbfc88eed91ce34b1f4d27e7eb7ca839)

Conflicts:
      modules.d/01fips-crypto-policies/module-setup.sh
      Due to upstream e6117b92fa0108dbaf9ea3ac0ec8f5a02487c812, which
      was not cherry-picked. Resolved the conflict by keeping the
      functions (i.e., undoing the cleanup of the upstream commit).
@neverpanic
Copy link
Contributor Author

I rebased my three commits on top of main.

@pvalena
Copy link
Contributor

pvalena commented Sep 19, 2024

Thanks! I'll merge this right after fixing the CI (in hour or two).

@pvalena
Copy link
Contributor

pvalena commented Sep 19, 2024

Sorry, I just realized you're changing the spec with entries and dist.
The idea is to have it handled automatically (either by custom automation or ideally by packit itself), and bundle the changes from commits to this repo to one commit to dist-git.
So I'll drop the dist-git changes, and merge without it. Otherwise it looks good, thanks!

#39

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants