From faaf21d2e5f1917939641781ac1da37e0141a9f8 Mon Sep 17 00:00:00 2001 From: Christian Glombek Date: Tue, 14 Jul 2020 17:57:46 +0200 Subject: [PATCH] Add kernel arg tuneable allowlist for FCOS This is required for Fedora CoreOS installs, which set `mitigations=auto,nosmt` and may enable cgroups v2 in the future so it has to be allowed to disable it explicitly. --- cmd/machine-config-daemon/pivot.go | 40 ++++++++++++++++++++++++------ pkg/daemon/daemon.go | 8 +++--- pkg/daemon/osrelease.go | 16 ++++++------ pkg/daemon/update.go | 16 ++++++------ pkg/daemon/update_test.go | 4 +-- 5 files changed, 55 insertions(+), 29 deletions(-) diff --git a/cmd/machine-config-daemon/pivot.go b/cmd/machine-config-daemon/pivot.go index 00247c376f..48221cc9bd 100644 --- a/cmd/machine-config-daemon/pivot.go +++ b/cmd/machine-config-daemon/pivot.go @@ -34,12 +34,18 @@ const ( cmdLineFile = "/proc/cmdline" ) -// TODO: fill out the allowlist -// tuneableArgsAllowlist contains allowed keys for tunable arguments -var tuneableArgsAllowlist = map[string]bool{ +// TODO: fill out the allowlists +// tuneableRHCOSArgsAllowlist contains allowed keys for tunable kernel arguments on RHCOS +var tuneableRHCOSArgsAllowlist = map[string]bool{ "nosmt": true, } +// tuneableFCOSArgsAllowlist contains allowed keys for tunable kernel arguments on FCOS +var tuneableFCOSArgsAllowlist = map[string]bool{ + "systemd.unified_cgroup_hierarchy=0": true, + "mitigations=auto,nosmt": true, +} + var pivotCmd = &cobra.Command{ Use: "pivot", DisableFlagsInUseLine: true, @@ -57,8 +63,20 @@ func init() { } // isArgTuneable returns if the argument provided is allowed to be modified -func isArgTunable(arg string) bool { - return tuneableArgsAllowlist[arg] +func isArgTunable(arg string) (bool, error) { + operatingSystem, err := daemon.GetHostRunningOS() + if err != nil { + return false, errors.Errorf("failed to get OS for determining whether kernel arg is tuneable: %v", err) + } + + switch operatingSystem { + case daemon.MachineConfigDaemonOSRHCOS: + return tuneableRHCOSArgsAllowlist[arg], nil + case daemon.MachineConfigDaemonOSFCOS: + return tuneableFCOSArgsAllowlist[arg], nil + default: + return false, nil + } } // isArgInUse checks to see if the argument is already in use by the system currently @@ -108,7 +126,11 @@ func parseTuningFile(tuningFilePath, cmdLinePath string) ([]types.TuneArgument, // NOTE: Today only specific bare kernel arguments are allowed so // there is not a need to split on =. key := strings.TrimSpace(line[len("ADD "):]) - if isArgTunable(key) { + tuneableKarg, err := isArgTunable(key) + if err != nil { + return addArguments, deleteArguments, err + } + if tuneableKarg { // Find out if the argument is in use inUse, err := isArgInUse(key, cmdLinePath) if err != nil { @@ -126,7 +148,11 @@ func parseTuningFile(tuningFilePath, cmdLinePath string) ([]types.TuneArgument, // NOTE: Today only specific bare kernel arguments are allowed so // there is not a need to split on =. key := strings.TrimSpace(line[len("DELETE "):]) - if isArgTunable(key) { + tuneableKarg, err := isArgTunable(key) + if err != nil { + return addArguments, deleteArguments, err + } + if tuneableKarg { inUse, err := isArgInUse(key, cmdLinePath) if err != nil { return addArguments, deleteArguments, err diff --git a/pkg/daemon/daemon.go b/pkg/daemon/daemon.go index fdf46ea6a3..57360921d4 100644 --- a/pkg/daemon/daemon.go +++ b/pkg/daemon/daemon.go @@ -204,7 +204,7 @@ func New( operatingSystem := "mock" if !mock { - operatingSystem, err = getHostRunningOS() + operatingSystem, err = GetHostRunningOS() if err != nil { HostOS.WithLabelValues("unsupported", "").Set(1) return nil, errors.Wrapf(err, "checking operating system") @@ -212,7 +212,7 @@ func New( } // Only pull the osImageURL from OSTree when we are on RHCOS or FCOS - if operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS { + if operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS { osImageURL, osVersion, err = nodeUpdaterClient.GetBootedOSImageURL() if err != nil { return nil, fmt.Errorf("error reading osImageURL from rpm-ostree: %v", err) @@ -827,7 +827,7 @@ func (dn *Daemon) getStateAndConfigs(pendingConfigName string) (*stateAndConfigs // dynamically after a reboot. func (dn *Daemon) LogSystemData() { // Print status if available - if dn.OperatingSystem == machineConfigDaemonOSRHCOS || dn.OperatingSystem == machineConfigDaemonOSFCOS { + if dn.OperatingSystem == MachineConfigDaemonOSRHCOS || dn.OperatingSystem == MachineConfigDaemonOSFCOS { status, err := dn.NodeUpdaterClient.GetStatus() if err != nil { glog.Fatalf("unable to get rpm-ostree status: %s", err) @@ -1369,7 +1369,7 @@ func compareOSImageURL(current, desired string) (bool, error) { // Otherwise if `false` is returned, then we need to perform an update. func (dn *Daemon) checkOS(osImageURL string) (bool, error) { // Nothing to do if we're not on RHCOS or FCOS - if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS { + if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS { glog.Infof(`Not booted into a CoreOS variant, ignoring target OSImageURL %s`, osImageURL) return true, nil } diff --git a/pkg/daemon/osrelease.go b/pkg/daemon/osrelease.go index eea4b454dc..525f5c7746 100644 --- a/pkg/daemon/osrelease.go +++ b/pkg/daemon/osrelease.go @@ -7,21 +7,21 @@ import ( ) const ( - // machineConfigDaemonOSRHCOS denotes RHEL CoreOS - machineConfigDaemonOSRHCOS = "RHCOS" + // MachineConfigDaemonOSRHCOS denotes RHEL CoreOS + MachineConfigDaemonOSRHCOS = "RHCOS" // machineConfigDaemonOSRHEL denotes RHEL machineConfigDaemonOSRHEL = "RHEL" // machineConfigDaemonOSCENTOS denotes CentOS machineConfigDaemonOSCENTOS = "CENTOS" - // machineConfigDaemonOSFCOS denotes Fedora CoreOS - machineConfigDaemonOSFCOS = "FCOS" + // MachineConfigDaemonOSFCOS denotes Fedora CoreOS + MachineConfigDaemonOSFCOS = "FCOS" ) -// getHostRunningOS reads os-release from the rootFs prefix to return what +// GetHostRunningOS reads os-release from the rootFs prefix to return what // OS variant the daemon is running on. If we are unable to read the // os-release file OR the information doesn't match MCD supported OS's // an error is returned. -func getHostRunningOS() (string, error) { +func GetHostRunningOS() (string, error) { libPath := "/usr/lib/os-release" etcPath := "/etc/os-release" @@ -31,13 +31,13 @@ func getHostRunningOS() (string, error) { } if or.ID == "fedora" && or.VARIANT_ID == "coreos" { - return machineConfigDaemonOSFCOS, nil + return MachineConfigDaemonOSFCOS, nil } // See https://github.com/openshift/redhat-release-coreos/blob/master/redhat-release-coreos.spec switch or.ID { case "rhcos": - return machineConfigDaemonOSRHCOS, nil + return MachineConfigDaemonOSRHCOS, nil case "rhel": return machineConfigDaemonOSRHEL, nil case "centos": diff --git a/pkg/daemon/update.go b/pkg/daemon/update.go index a61097f3b6..c052e099ea 100644 --- a/pkg/daemon/update.go +++ b/pkg/daemon/update.go @@ -689,7 +689,7 @@ func (dn *Daemon) updateKernelArguments(oldConfig, newConfig *mcfgv1.MachineConf if len(diff) == 0 { return nil } - if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS { + if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS { return fmt.Errorf("Updating kargs on non-CoreOS nodes is not supported: %v", diff) } @@ -739,7 +739,7 @@ func (dn *Daemon) switchKernel(oldConfig, newConfig *mcfgv1.MachineConfig) error return nil } // We support Kernel update only on RHCOS nodes - if dn.OperatingSystem != machineConfigDaemonOSRHCOS { + if dn.OperatingSystem != MachineConfigDaemonOSRHCOS { return fmt.Errorf("Updating kernel on non-RHCOS nodes is not supported") } @@ -906,7 +906,7 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ign3types.Config) newFileSet[f.Path] = struct{}{} } - operatingSystem, err := getHostRunningOS() + operatingSystem, err := GetHostRunningOS() if err != nil { return errors.Wrapf(err, "checking operating system") } @@ -926,7 +926,7 @@ func (dn *Daemon) deleteStaleData(oldIgnConfig, newIgnConfig *ign3types.Config) if _, err := exec.Command("rpm", "-qf", f.Path).CombinedOutput(); err == nil { // File is owned by an rpm restore = true - } else if strings.HasPrefix(f.Path, "/etc") && (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { + } else if strings.HasPrefix(f.Path, "/etc") && (operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) { if _, err := os.Stat("/usr" + f.Path); err != nil { if !os.IsNotExist(err) { return err @@ -1071,7 +1071,7 @@ func (dn *Daemon) disableUnit(unit ign3types.Unit) error { // writeUnits writes the systemd units to disk func (dn *Daemon) writeUnits(units []ign3types.Unit) error { - operatingSystem, err := getHostRunningOS() + operatingSystem, err := GetHostRunningOS() if err != nil { return errors.Wrapf(err, "checking operating system") } @@ -1081,7 +1081,7 @@ func (dn *Daemon) writeUnits(units []ign3types.Unit) error { glog.Infof("Writing systemd unit dropin %q", u.Dropins[i].Name) dpath := filepath.Join(pathSystemd, u.Name+".d", u.Dropins[i].Name) if _, err := os.Stat("/usr" + dpath); err == nil && - (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { + (operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) { if err := createOrigFile("/usr"+dpath, dpath); err != nil { return err } @@ -1115,7 +1115,7 @@ func (dn *Daemon) writeUnits(units []ign3types.Unit) error { if u.Contents != nil && *u.Contents != "" { glog.Infof("Writing systemd unit %q", u.Name) if _, err := os.Stat("/usr" + fpath); err == nil && - (operatingSystem == machineConfigDaemonOSRHCOS || operatingSystem == machineConfigDaemonOSFCOS) { + (operatingSystem == MachineConfigDaemonOSRHCOS || operatingSystem == MachineConfigDaemonOSFCOS) { if err := createOrigFile("/usr"+fpath, fpath); err != nil { return err } @@ -1300,7 +1300,7 @@ func (dn *Daemon) updateSSHKeys(newUsers []ign3types.PasswdUser) error { // updateOS updates the system OS to the one specified in newConfig func (dn *Daemon) updateOS(config *mcfgv1.MachineConfig) error { - if dn.OperatingSystem != machineConfigDaemonOSRHCOS && dn.OperatingSystem != machineConfigDaemonOSFCOS { + if dn.OperatingSystem != MachineConfigDaemonOSRHCOS && dn.OperatingSystem != MachineConfigDaemonOSFCOS { glog.V(2).Info("Updating of non-CoreOS nodes are not supported") return nil } diff --git a/pkg/daemon/update_test.go b/pkg/daemon/update_test.go index 9b12216220..ccd3ce3f0f 100644 --- a/pkg/daemon/update_test.go +++ b/pkg/daemon/update_test.go @@ -36,7 +36,7 @@ func TestUpdateOS(t *testing.T) { d := Daemon{ mock: true, name: "nodeName", - OperatingSystem: machineConfigDaemonOSRHCOS, + OperatingSystem: MachineConfigDaemonOSRHCOS, NodeUpdaterClient: testClient, kubeClient: k8sfake.NewSimpleClientset(), bootedOSImageURL: "test", @@ -387,7 +387,7 @@ func TestUpdateSSHKeys(t *testing.T) { d := Daemon{ mock: true, name: "nodeName", - OperatingSystem: machineConfigDaemonOSRHCOS, + OperatingSystem: MachineConfigDaemonOSRHCOS, NodeUpdaterClient: testClient, kubeClient: k8sfake.NewSimpleClientset(), bootedOSImageURL: "test",