From affa37c7329ab27baac4b7002ed21465953dec42 Mon Sep 17 00:00:00 2001 From: Clemens Lang Date: Wed, 25 Sep 2024 13:14:31 +0200 Subject: [PATCH] pkg/subscriptions: Modernize FIPS mounts /etc/system-fips is deprecated in CentOS Stream 9 and has been removed from CentOS Stream 10. UBI8 containers still contain /etc/system-fips -> /run/secrets/system-fips, but UBI9 containers do not, so creating /run/secrets/system-fips on UBI9 (or later) does not serve a useful purpose. See [1, 2]. Instead of checking /etc/system-fips to determine whether FIPS mode is enabled on the host, read /proc/sys/crypto/fips_enabled, which works for all supported RHEL versions and likely even earlier. In CentOS 10 Stream, the crypto-policies package does now contain /usr/share/crypto-policies/default-fips-config, which is meant to serve as a file to bind-mount over /etc/crypto-policies/config when in FIPS mode [3]. Manual creation of this file is thus no longer required in containers/common for modern containers. Using this file as a source also enables improvements in crypto-policies tooling which will now - unmount the two bind mounts when a user manually changes the policy using update-crypto-policies --set, something which was previously broken in containers because /etc/crypto-policies/config was a read-only bind-mount, and - unmount and restore the two bind-mounts when the crypto-policies package is updated. The crypto-policies package will only do these steps if the the bind mounts for crypto-policies use the /usr/share/crypto-policies/default-fips-config file as source, so it makes sense for containers/common to switch to that. [1]: https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/merge_requests/111 [2]: https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/9/html/9.0_release_notes/deprecated_functionality#deprecated-functionality_security [3]: https://gitlab.com/redhat-crypto/fedora-crypto-policies/-/commit/04ceadccfc07e5946b08157d06ca5c0d5a229d92 Closes: containers/common#2130 Related: https://issues.redhat.com/browse/CRYPTO-13556 Signed-off-by: Clemens Lang --- pkg/subscriptions/subscriptions.go | 203 ++++++++++++++++++++--------- 1 file changed, 142 insertions(+), 61 deletions(-) diff --git a/pkg/subscriptions/subscriptions.go b/pkg/subscriptions/subscriptions.go index a6538ffb9..f93967045 100644 --- a/pkg/subscriptions/subscriptions.go +++ b/pkg/subscriptions/subscriptions.go @@ -155,6 +155,27 @@ func getMountsMap(path string) (string, string, error) { //nolint return "", "", fmt.Errorf("unable to get host and container dir from path: %s", path) } +// Return true iff the system is in FIPS mode as determined by reading +// /proc/sys/crypto/fips_enabled and disableFips is not true. +func shouldAddFIPSMounts(disableFips bool) bool { + if disableFips { + return false + } + + fips_enabled, err := os.ReadFile("/proc/sys/crypto/fips_enabled") + if err != nil { + logrus.Errorf("Failed to read /proc/sys/crypto/fips_enabled to determine FIPS state: %v", err) + return false + } + + if strings.TrimSpace(string(fips_enabled)) != "1" { + logrus.Debug("/proc/sys/crypto/fips_enabled does not contain '1', not adding FIPS mode bind mounts") + return false + } + + return true +} + // MountsWithUIDGID copies, adds, and mounts the subscriptions to the container root filesystem // mountLabel: MAC/SELinux label for container content // containerRunDir: Private data for storing subscriptions on the host mounted in container. @@ -194,22 +215,16 @@ func MountsWithUIDGID(mountLabel, containerRunDir, mountFile, mountPoint string, } } - // Only add FIPS subscription mount if disableFips=false - if disableFips { + // Only add FIPS subscription mount if disableFips=false and + // /proc/sys/cryto/fips_enabled contains "1" + if !shouldAddFIPSMounts(disableFips) { return subscriptionMounts } - // Add FIPS mode subscription if /etc/system-fips exists on the host - err := fileutils.Exists("/etc/system-fips") - switch { - case err == nil: - if err := addFIPSModeSubscription(&subscriptionMounts, containerRunDir, mountPoint, mountLabel, uid, gid); err != nil { - logrus.Errorf("Adding FIPS mode subscription to container: %v", err) - } - case errors.Is(err, os.ErrNotExist): - logrus.Debug("/etc/system-fips does not exist on host, not mounting FIPS mode subscription") - default: - logrus.Errorf("stat /etc/system-fips failed for FIPS mode subscription: %v", err) + + if err := addFIPSMounts(&subscriptionMounts, containerRunDir, mountPoint, mountLabel, uid, gid); err != nil { + logrus.Errorf("Adding FIPS mode bind mounts to container: %v", err) } + return subscriptionMounts } @@ -306,43 +321,92 @@ func addSubscriptionsFromMountsFile(filePath, mountLabel, containerRunDir string return mounts, nil } -// addFIPSModeSubscription adds mounts to the `mounts` slice that are needed for the container to run openssl in FIPs mode -// (i.e: be FIPs compliant). -// It should only be called if /etc/system-fips exists on host. -// It primarily does two things: -// - creates /run/secrets/system-fips in the container root filesystem, and adds it to the `mounts` slice. -// - If `/etc/crypto-policies/back-ends` already exists inside of the container, it creates -// `/usr/share/crypto-policies/back-ends/FIPS` inside the container as well. -// It is done from within the container to ensure to avoid policy incompatibility between the container and host. -func addFIPSModeSubscription(mounts *[]rspec.Mount, containerRunDir, mountPoint, mountLabel string, uid, gid int) error { +func containerHasEtcSystemFips(subscriptionsDir, mountPoint string) (bool, error) { + fipsFileHost := filepath.Join(mountPoint, "etc/system-fips") + if fileutils.Lexists(fipsFileHost) != nil { + logrus.Debug("/etc/system-fips does not exist in the container, not creating /run/secrets/system-fips") + return false, nil + } + + fipsFileTarget, err := securejoin.SecureJoin(mountPoint, "etc/system-fips") + if err != nil { + return false, fmt.Errorf("Container /etc/system-fips resolution error: %w", err) + } + if fipsFileTarget != filepath.Join(mountPoint, subscriptionsDir, "system-fips") { + logrus.Warn(fmt.Sprintf("/etc/system-fips exists in the container, but is not a bind-mount to %[1]v/system-fips; not creating %[1]v/system-fips", subscriptionsDir)) + return false, nil + } + + return true, nil +} + +// addFIPSMounts adds mounts to the `mounts` slice that are needed +// for the container to run cryptographic libraries (openssl, gnutls, NSS, ...) +// in FIPS mode (i.e: be FIPS compliant). +// It should only be called if /proc/sys/crypto/fips_enabled on the host +// contains '1'. +// It does three things: +// - creates /run/secrets/system-fips in the container root filesystem if +// /etc/system-fips exists and is a symlink to /run/secrets/system-fips, +// and adds it to the `mounts` slice. This is, for example, the case on +// RHEL 8, but not on newer RHEL, since /etc/system-fips is deprecated. +// - Bind-mounts `/usr/share/crypto-policies/back-ends/FIPS` over +// `/etc/crypto-policies/back-ends` if the former exists inside of the +// container. This is done from within the container to avoid policy +// incompatibility between container and host. +// - If a bind mount for `/etc/crypto-policies/back-ends` was created, +// bind-mounts `/usr/share/crypto-policies/default-fips-config` over +// `/etc/crypto-policies/config` if the former exists inside of the +// container. If it does not exist, creates a new temporary file containing +// "FIPS\n", and bind-mounts that over `/etc/crypto-policies/config`. +// +// Starting in CentOS 10 Stream, the crypto-policies package gracefully recognizes the two bind mounts +// /etc/crypto-policies/config -> /usr/share/crypto-policies/default-fips-config +// /etc/crypto-policies/back-ends/FIPS -> /usr/share/crypto-policies/back-ends/FIPS +// and unmounts them when users manually change the policy, or removes and +// restores the mounts when the crypto-policies package is upgraded. +func addFIPSMounts(mounts *[]rspec.Mount, containerRunDir, mountPoint, mountLabel string, uid, gid int) error { + // Check whether $container/etc/system-fips exists and is a symlink to /run/secrets/system-fips subscriptionsDir := "/run/secrets" - ctrDirOnHost := filepath.Join(containerRunDir, subscriptionsDir) - if err := fileutils.Exists(ctrDirOnHost); errors.Is(err, os.ErrNotExist) { - if err = idtools.MkdirAllAs(ctrDirOnHost, 0o755, uid, gid); err != nil { //nolint - return err - } - if err = label.Relabel(ctrDirOnHost, mountLabel, false); err != nil { - return fmt.Errorf("applying correct labels on %q: %w", ctrDirOnHost, err) - } + + createSystemFipsSecret, err := containerHasEtcSystemFips(subscriptionsDir, mountPoint) + if err != nil { + return err } - fipsFile := filepath.Join(ctrDirOnHost, "system-fips") - // In the event of restart, it is possible for the FIPS mode file to already exist - if err := fileutils.Exists(fipsFile); errors.Is(err, os.ErrNotExist) { - file, err := os.Create(fipsFile) - if err != nil { - return fmt.Errorf("creating system-fips file in container for FIPS mode: %w", err) + if createSystemFipsSecret { + // This container contains + // /etc/system-fips -> /run/secrets/system-fips + // and expects podman to create this file if the container should + // be in FIPS mode + ctrDirOnHost := filepath.Join(containerRunDir, subscriptionsDir) + if err := fileutils.Exists(ctrDirOnHost); errors.Is(err, os.ErrNotExist) { + if err = idtools.MkdirAllAs(ctrDirOnHost, 0o755, uid, gid); err != nil { //nolint + return err + } + if err = label.Relabel(ctrDirOnHost, mountLabel, false); err != nil { + return fmt.Errorf("applying correct labels on %q: %w", ctrDirOnHost, err) + } } - file.Close() - } + fipsFile := filepath.Join(ctrDirOnHost, "system-fips") - if !mountExists(*mounts, subscriptionsDir) { - m := rspec.Mount{ - Source: ctrDirOnHost, - Destination: subscriptionsDir, - Type: "bind", - Options: []string{"bind", "rprivate"}, + // In the event of restart, it is possible for the FIPS mode file to already exist + if err := fileutils.Exists(fipsFile); errors.Is(err, os.ErrNotExist) { + file, err := os.Create(fipsFile) + if err != nil { + return fmt.Errorf("creating system-fips file in container for FIPS mode: %w", err) + } + file.Close() + } + + if !mountExists(*mounts, subscriptionsDir) { + m := rspec.Mount{ + Source: ctrDirOnHost, + Destination: subscriptionsDir, + Type: "bind", + Options: []string{"bind", "rprivate"}, + } + *mounts = append(*mounts, m) } - *mounts = append(*mounts, m) } srcBackendDir := "/usr/share/crypto-policies/back-ends/FIPS" @@ -370,27 +434,44 @@ func addFIPSModeSubscription(mounts *[]rspec.Mount, containerRunDir, mountPoint, // Make sure we set the config to FIPS so that the container does not overwrite // /etc/crypto-policies/back-ends when crypto-policies-scripts is reinstalled. - cryptoPoliciesConfigFile := filepath.Join(containerRunDir, "fips-config") - file, err := os.Create(cryptoPoliciesConfigFile) + // + // Starting in CentOS 10 Stream, crypto-policies provides + // /usr/share/crypto-policies/default-fips-config as bind mount source + // file and the crypto-policies tooling gracefully deals with the two bind-mounts + // /etc/crypto-policies/back-ends -> /usr/share/crypto-policies/back-ends/FIPS + // /etc/crypto-policies/config -> /usr/share/crypto-policies/default-fips-config + // if they both exist. + srcPolicyConfig := "/usr/share/crypto-policies/default-fips-config" + destPolicyConfig := "/etc/crypto-policies/config" + srcPolicyConfigOnHost, err := securejoin.SecureJoin(mountPoint, srcPolicyConfig) if err != nil { - return fmt.Errorf("creating fips config file in container for FIPS mode: %w", err) - } - defer file.Close() - if _, err := file.WriteString("FIPS\n"); err != nil { - return fmt.Errorf("writing fips config file in container for FIPS mode: %w", err) - } - if err = label.Relabel(cryptoPoliciesConfigFile, mountLabel, false); err != nil { - return fmt.Errorf("applying correct labels on fips-config file: %w", err) + return fmt.Errorf("Could not expand %q in container: %w", srcPolicyConfig, err) } - if err := file.Chown(uid, gid); err != nil { - return fmt.Errorf("chown fips-config file: %w", err) + + if err = fileutils.Exists(srcPolicyConfigOnHost); err != nil { + if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("Could not check whether %q exists in container: %w", srcPolicyConfig, err) + } + + // /usr/share/crypto-policies/default-fips-config does not exist, let's create it ourselves + cryptoPoliciesConfigFile := filepath.Join(containerRunDir, "fips-config") + if err := os.WriteFile(cryptoPoliciesConfigFile, []byte("FIPS\n"), 0o644); err != nil { + return fmt.Errorf("Failed to write fips config file in container for FIPS mode: %w", err) + } + if err = label.Relabel(cryptoPoliciesConfigFile, mountLabel, false); err != nil { + return fmt.Errorf("Failed to apply correct labels on fips config file: %w", err) + } + if err := os.Chown(cryptoPoliciesConfigFile, uid, gid); err != nil { + return fmt.Errorf("Failed to chown fips config file: %w", err) + } + + srcPolicyConfigOnHost = cryptoPoliciesConfigFile } - policyConfig := "/etc/crypto-policies/config" - if !mountExists(*mounts, policyConfig) { + if !mountExists(*mounts, destPolicyConfig) { m := rspec.Mount{ - Source: cryptoPoliciesConfigFile, - Destination: policyConfig, + Source: srcPolicyConfigOnHost, + Destination: destPolicyConfig, Type: "bind", Options: []string{"bind", "rprivate"}, }