Merge pull request kubevirt#9048 from jean-edouard/selinuxfg

Introduce a feature gate to disable the custom SELinux policy

cmd/virt-handler/virt-handler.go

@@ -29,6 +29,7 @@ import (
 	"os/signal"
 	"path/filepath"
 	"runtime"
+	"sync"
 	"syscall"
 	"time"
 
@@ -142,6 +143,10 @@ type virtHandlerApp struct {
 	domainResyncPeriodSeconds int
 	gracefulShutdownSeconds   int
 
+	// Remember whether we have already installed the custom SELinux policy or not
+	customSELinuxPolicyInstalled bool
+	semoduleLock                 sync.Mutex
+
 	caConfigMapName    string
 	clientCertFilePath string
 	clientKeyFilePath  string
@@ -303,6 +308,7 @@ func (app *virtHandlerApp) Run() {
 	// set log verbosity
 	app.clusterConfig.SetConfigModifiedCallback(app.shouldChangeLogVerbosity)
 	app.clusterConfig.SetConfigModifiedCallback(app.shouldChangeRateLimiter)
+	app.clusterConfig.SetConfigModifiedCallback(app.shouldInstallSELinuxPolicy)
 
 	if err := app.setupTLS(factory); err != nil {
 		glog.Fatalf("Error constructing migration tls config: %v", err)
@@ -392,13 +398,6 @@ func (app *virtHandlerApp) Run() {
 
 	se, exists, err := selinux.NewSELinux()
 	if err == nil && exists {
-		log.DefaultLogger().Infof("SELinux is reported as '%s'", se.Mode())
-		// Install KubeVirt's virt-launcher policy
-		err = se.InstallPolicy("/var/run/kubevirt")
-		if err != nil {
-			panic(fmt.Errorf("failed to install virt-launcher selinux policy: %v", err))
-		}
-
 		// relabel tun device
 		unprivilegedContainerSELinuxLabel := "system_u:object_r:container_file_t:s0"
 
@@ -480,6 +479,15 @@ func (app *virtHandlerApp) Run() {
 	}
 }
 
+func (app *virtHandlerApp) installCustomSELinuxPolicy(se selinux.SELinux) {
+	// Install KubeVirt's virt-launcher policy
+	err := se.InstallPolicy("/var/run/kubevirt")
+	if err != nil {
+		panic(fmt.Errorf("failed to install virt-launcher selinux policy: %v", err))
+	}
+	app.customSELinuxPolicyInstalled = true
+}
+
 // Update virt-handler log verbosity on relevant config changes
 func (app *virtHandlerApp) shouldChangeLogVerbosity() {
 	verbosity := app.clusterConfig.GetVirtHandlerVerbosity(app.HostOverride)
@@ -496,6 +504,21 @@ func (app *virtHandlerApp) shouldChangeRateLimiter() {
 	log.Log.V(2).Infof("setting rate limiter to %v QPS and %v Burst", qps, burst)
 }
 
+// Install the SELinux policy when the feature gate that disables it gets removed
+func (app *virtHandlerApp) shouldInstallSELinuxPolicy() {
+	app.semoduleLock.Lock()
+	defer app.semoduleLock.Unlock()
+	if app.customSELinuxPolicyInstalled {
+		return
+	}
+	if !app.clusterConfig.CustomSELinuxPolicyDisabled() {
+		se, exists, err := selinux.NewSELinux()
+		if err == nil && exists {
+			app.installCustomSELinuxPolicy(se)
+		}
+	}
+}
+
 func (app *virtHandlerApp) runPrometheusServer(errCh chan error) {
 	mux := restful.NewContainer()
 	webService := new(restful.WebService)

docs/getting-started.md

@@ -52,6 +52,18 @@ login`. Your `$HOME/.docker/config.json` should look like:
 
 SELinux-enabled nodes need to have [Container-selinux](https://github.com/containers/container-selinux) version 2.170.0 or newer installed.
 
+#### Disabling the custom SELinux policy
+
+By default, a custom SELinux policy gets installed by virt-handler on every node, and it gets used for VMIs that need it.
+Currently, the only VMIs using it are the ones that enable passt-based networking.  
+However, having KubeVirt install and use a custom SELinux policy is a security concern. It also increases virt-handler boot time 20/30 seconds.  
+Therefore, a feature gate was introduced to disable the installation and usage of that custom SELinux policy: `DisableCustomSELinuxPolicy`.  
+The side effect is that passt-enabled VMIs will fail to start, but only on nodes that use container-selinux version 2.192.0 or lower.  
+container-selinux releases 2.193.0 and newer include the necessary permissions for passt-enabled VMIs to run successfully.
+
+**Note:** adding the `DisableCustomSELinuxPolicy` feature gate to an existing cluster will disable the use of the custom policy for new VMIs,
+but will **not** automatically uninstall the policy from the nodes. That can be done manually if needed, by running `semodule -r virt_launcher` on every node.
+
 ## Building
 
 The KubeVirt build system runs completely inside Docker. 

pkg/virt-config/feature-gates.go

@@ -55,6 +55,8 @@ const (
 	VSOCKGate                  = "VSOCK"
 	// PSASeccompAllowsUserfaultfd tells us that the seccomp policy on the nodes allow the userfaultfd syscall, which is needed for post-copy migrations
 	PSASeccompAllowsUserfaultfd = "PSASeccompAllowsUserfaultfd"
+	// DisableCustomSELinuxPolicy disables the installation of the custom SELinux policy for virt-launcher
+	DisableCustomSELinuxPolicy = "DisableCustomSELinuxPolicy"
 )
 
 var deprecatedFeatureGates = [...]string{
@@ -65,22 +67,22 @@ var deprecatedFeatureGates = [...]string{
 	PSA,
 }
 
-func (c *ClusterConfig) isFeatureGateEnabled(featureGate string) bool {
-	if c.IsFeatureGateDeprecated(featureGate) {
+func (config *ClusterConfig) isFeatureGateEnabled(featureGate string) bool {
+	if config.IsFeatureGateDeprecated(featureGate) {
 		// Deprecated feature gates are considered enabled and no-op.
 		// For more info about deprecation policy: https://github.com/kubevirt/kubevirt/blob/main/docs/deprecation.md
 		return true
 	}
 
-	for _, fg := range c.GetConfig().DeveloperConfiguration.FeatureGates {
+	for _, fg := range config.GetConfig().DeveloperConfiguration.FeatureGates {
 		if fg == featureGate {
 			return true
 		}
 	}
 	return false
 }
 
-func (c *ClusterConfig) IsFeatureGateDeprecated(featureGate string) bool {
+func (config *ClusterConfig) IsFeatureGateDeprecated(featureGate string) bool {
 	for _, deprecatedFeatureGate := range deprecatedFeatureGates {
 		if featureGate == deprecatedFeatureGate {
 			return true
@@ -189,3 +191,7 @@ func (config *ClusterConfig) VSOCKEnabled() bool {
 func (config *ClusterConfig) PSASeccompAllowsUserfaultfd() bool {
 	return config.isFeatureGateEnabled(PSASeccompAllowsUserfaultfd)
 }
+
+func (config *ClusterConfig) CustomSELinuxPolicyDisabled() bool {
+	return config.isFeatureGateEnabled(DisableCustomSELinuxPolicy)
+}

pkg/virt-controller/services/template.go

@@ -536,7 +536,7 @@ func (t *templateService) renderLaunchManifest(vmi *v1.VirtualMachineInstance, i
 		},
 	}
 
-	alignPodMultiCategorySecurity(&pod, vmi, t.clusterConfig.GetSELinuxLauncherType(), t.clusterConfig.DockerSELinuxMCSWorkaroundEnabled())
+	alignPodMultiCategorySecurity(&pod, vmi, t.clusterConfig.GetSELinuxLauncherType(), t.clusterConfig.DockerSELinuxMCSWorkaroundEnabled(), t.clusterConfig.CustomSELinuxPolicyDisabled())
 
 	// If we have a runtime class specified, use it, otherwise don't set a runtimeClassName
 	runtimeClassName := t.clusterConfig.GetDefaultRuntimeClass()
@@ -1166,9 +1166,9 @@ func wrapGuestAgentPingWithVirtProbe(vmi *v1.VirtualMachineInstance, probe *k8sv
 	return
 }
 
-func alignPodMultiCategorySecurity(pod *k8sv1.Pod, vmi *v1.VirtualMachineInstance, selinuxType string, dockerSELinuxMCSWorkaround bool) {
+func alignPodMultiCategorySecurity(pod *k8sv1.Pod, vmi *v1.VirtualMachineInstance, selinuxType string, dockerSELinuxMCSWorkaround bool, customPolicyDisabled bool) {
 	if selinuxType == "" {
-		if util.IsPasstVMI(vmi) {
+		if !customPolicyDisabled && util.IsPasstVMI(vmi) {
 			// If no SELinux type was specified, use our custom type for VMIs that need it
 			selinuxType = customSELinuxType
 		}
@@ -1186,16 +1186,18 @@ func alignPodMultiCategorySecurity(pod *k8sv1.Pod, vmi *v1.VirtualMachineInstanc
 		pod.Spec.SecurityContext.SELinuxOptions = &k8sv1.SELinuxOptions{Type: selinuxType}
 	}
 
-	// more info on https://github.com/kubernetes/kubernetes/issues/90759
-	// Since the compute container needs to be able to communicate with the
-	// rest of the pod, we loop over all the containers and remove their SELinux
-	// categories.
-	// This currently only affects Docker + SELinux use-cases, and requires a
-	// feature gate to be set.
-	for i := range pod.Spec.Containers {
-		container := &pod.Spec.Containers[i]
-		if container.Name != "compute" {
-			generateContainerSecurityContext(selinuxType, container, dockerSELinuxMCSWorkaround)
+	if dockerSELinuxMCSWorkaround {
+		// more info on https://github.com/kubernetes/kubernetes/issues/90759
+		// Since the compute container needs to be able to communicate with the
+		// rest of the pod, we loop over all the containers and remove their SELinux
+		// categories.
+		// This currently only affects Docker + SELinux use-cases, and requires a
+		// feature gate to be set.
+		for i := range pod.Spec.Containers {
+			container := &pod.Spec.Containers[i]
+			if container.Name != "compute" {
+				generateContainerSecurityContext(selinuxType, container)
+			}
 		}
 	}
 }
@@ -1214,17 +1216,15 @@ func matchSELinuxLevelOfVMI(pod *k8sv1.Pod, vmi *v1.VirtualMachineInstance) erro
 	return nil
 }
 
-func generateContainerSecurityContext(selinuxType string, container *k8sv1.Container, forceLevel bool) {
+func generateContainerSecurityContext(selinuxType string, container *k8sv1.Container) {
 	if container.SecurityContext == nil {
 		container.SecurityContext = &k8sv1.SecurityContext{}
 	}
 	if container.SecurityContext.SELinuxOptions == nil {
 		container.SecurityContext.SELinuxOptions = &k8sv1.SELinuxOptions{}
 	}
 	container.SecurityContext.SELinuxOptions.Type = selinuxType
-	if forceLevel {
-		container.SecurityContext.SELinuxOptions.Level = "s0"
-	}
+	container.SecurityContext.SELinuxOptions.Level = "s0"
 }
 
 func generatePodAnnotations(vmi *v1.VirtualMachineInstance) (map[string]string, error) {

tests/security_features_test.go

@@ -23,6 +23,10 @@ import (
 	"context"
 	"fmt"
 	"strings"
+	"time"
+
+	"kubevirt.io/kubevirt/pkg/virt-operator/resource/generate/components"
+	"kubevirt.io/kubevirt/tests/libnode"
 
 	"kubevirt.io/kubevirt/tests/decorators"
 
@@ -274,8 +278,65 @@ var _ = Describe("[Serial][sig-compute]SecurityFeatures", Serial, decorators.Sig
 			}
 		})
 	})
+	Context("Disabling the custom SELinux policy", func() {
+		var policyRemoved = false
+		AfterEach(func() {
+			if policyRemoved {
+				By("Re-installing custom SELinux policy on all nodes")
+				err = runOnAllNodes(virtClient, []string{"cp", "/var/run/kubevirt/virt_launcher.cil", "/proc/1/root/tmp/"}, "")
+				Expect(err).ToNot(HaveOccurred())
+				err = runOnAllNodes(virtClient, []string{"chroot", "/proc/1/root", "semodule", "-i", "/tmp/virt_launcher.cil"}, "")
+				Expect(err).ToNot(HaveOccurred())
+				err = runOnAllNodes(virtClient, []string{"rm", "-f", "/proc/1/root/tmp/virt_launcher.cil"}, "")
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			By("Re-enabling the custom policy by removing the corresponding feature gate")
+			tests.DisableFeatureGate(virtconfig.DisableCustomSELinuxPolicy)
+		})
+
+		It("Should prevent virt-handler from installing the custom policy", func() {
+			By("Removing custom SELinux policy from all nodes")
+			err = runOnAllNodes(virtClient, []string{"chroot", "/proc/1/root", "semodule", "-r", "virt_launcher"}, "")
+			Expect(err).ToNot(HaveOccurred())
+			policyRemoved = true
+
+			By("Disabling the custom policy by adding the corresponding feature gate")
+			tests.EnableFeatureGate(virtconfig.DisableCustomSELinuxPolicy)
+
+			By("Ensuring the custom SELinux policy is absent from all nodes")
+			Consistently(func() error {
+				return runOnAllNodes(virtClient, []string{"chroot", "/proc/1/root", "semodule", "-l"}, "virt_launcher")
+			}, 30*time.Second, 10*time.Second).Should(BeNil())
+		})
+	})
 })
 
+func runOnAllNodes(virtClient kubecli.KubevirtClient, command []string, forbiddenString string) error {
+	nodes, err := virtClient.CoreV1().Nodes().List(context.Background(), metav1.ListOptions{})
+	if err != nil {
+		return err
+	}
+	for _, node := range nodes.Items {
+		pod, err := libnode.GetVirtHandlerPod(virtClient, node.Name)
+		if err != nil {
+			return err
+		}
+		stdout, stderr, err := exec.ExecuteCommandOnPodWithResults(virtClient, pod, components.VirtHandlerName, command)
+		if err != nil {
+			_, _ = GinkgoWriter.Write([]byte(stderr))
+			return err
+		}
+		if forbiddenString != "" {
+			if strings.Contains(stdout, forbiddenString) {
+				return fmt.Errorf("found unexpected %s on node %s", forbiddenString, node.Name)
+			}
+		}
+	}
+
+	return nil
+}
+
 func isLauncherCapabilityValid(capability k8sv1.Capability) bool {
 	switch capability {
 	case