File 0009-Respect-the-feature-gate-when-exposing-SEV... of Package kubevirt

Overview Repositories Revisions Requests Users Attributes Meta

File 0009-Respect-the-feature-gate-when-exposing-SEV-device.patch of Package kubevirt (Revision 1de9bd49f2f0a02a72d854e59aa1ab66)

Currently displaying revision 1de9bd49f2f0a02a72d854e59aa1ab66 , Show latest

From bbd07f2c7a392f3b3e3b5e87733a1f8cf7904abc Mon Sep 17 00:00:00 2001
From: Vasiliy Ulyanov <vulyanov@suse.de>
Date: Thu, 14 Jul 2022 13:44:29 +0200
Subject: [PATCH 1/2] Respect the feature gate when exposing SEV device

Having /dev/sev in the list of permanently available devices may impose
a security risk as it can be requested and accessed by an arbitrary pod.
Stop advertizing the device when the corresponding feature gate is
disabled.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
---
 .../device-manager/device_controller.go       | 35 ++++++++++++++++---
 .../device-manager/device_controller_test.go  | 14 +++++---
 .../device-manager/mediated_device_test.go    |  2 +-
 .../mediated_devices_types_test.go            |  2 +-
 .../device-manager/pci_device_test.go         |  2 +-
 pkg/virt-handler/vm.go                        |  8 ++++-
 6 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/pkg/virt-handler/device-manager/device_controller.go b/pkg/virt-handler/device-manager/device_controller.go
index e56937fff..0b61ab5c1 100644
--- a/pkg/virt-handler/device-manager/device_controller.go
+++ b/pkg/virt-handler/device-manager/device_controller.go
@@ -105,7 +105,6 @@ func PermanentHostDevicePlugins(maxDevices int, permissions string) []Device {
 		"kvm":       "/dev/kvm",
 		"tun":       "/dev/net/tun",
 		"vhost-net": "/dev/vhost-net",
-		"sev":       "/dev/sev",
 	}
 
 	ret := make([]Device, 0, len(permanentDevicePluginPaths))
@@ -125,6 +124,8 @@ type DeviceController struct {
 	startedPlugins      map[string]controlledDevice
 	startedPluginsMutex sync.Mutex
 	host                string
+	maxDevices          int
+	permissions         string
 	backoff             []time.Duration
 	virtConfig          *virtconfig.ClusterConfig
 	stop                chan struct{}
@@ -132,7 +133,14 @@ type DeviceController struct {
 	clientset           k8scli.CoreV1Interface
 }
 
-func NewDeviceController(host string, permanentPlugins []Device, clusterConfig *virtconfig.ClusterConfig, clientset k8scli.CoreV1Interface) *DeviceController {
+func NewDeviceController(
+	host string,
+	maxDevices int,
+	permissions string,
+	permanentPlugins []Device,
+	clusterConfig *virtconfig.ClusterConfig,
+	clientset k8scli.CoreV1Interface,
+) *DeviceController {
 	permanentPluginsMap := make(map[string]Device, len(permanentPlugins))
 	for i := range permanentPlugins {
 		permanentPluginsMap[permanentPlugins[i].GetDeviceName()] = permanentPlugins[i]
@@ -142,6 +150,8 @@ func NewDeviceController(host string, permanentPlugins []Device, clusterConfig *
 		permanentPlugins: permanentPluginsMap,
 		startedPlugins:   map[string]controlledDevice{},
 		host:             host,
+		maxDevices:       maxDevices,
+		permissions:      permissions,
 		backoff:          []time.Duration{1 * time.Second, 2 * time.Second, 5 * time.Second, 10 * time.Second},
 		virtConfig:       clusterConfig,
 		mdevTypesManager: NewMDEVTypesManager(),
@@ -159,12 +169,29 @@ func (c *DeviceController) NodeHasDevice(devicePath string) bool {
 
 // updatePermittedHostDevicePlugins returns a slice of device plugins for permitted devices which are present on the node
 func (c *DeviceController) updatePermittedHostDevicePlugins() []Device {
+	var permittedDevices []Device
+
+	var featureGatedDevices = []struct {
+		Name      string
+		Path      string
+		IsAllowed func() bool
+	}{
+		{"sev", "/dev/sev", c.virtConfig.WorkloadEncryptionSEVEnabled},
+	}
+	for _, dev := range featureGatedDevices {
+		if dev.IsAllowed() {
+			permittedDevices = append(
+				permittedDevices,
+				NewGenericDevicePlugin(dev.Name, dev.Path, c.maxDevices, c.permissions, true),
+			)
+		}
+	}
+
 	hostDevs := c.virtConfig.GetPermittedHostDevices()
 	if hostDevs == nil {
-		return nil
+		return permittedDevices
 	}
 
-	var permittedDevices []Device
 	if len(hostDevs.PciHostDevices) != 0 {
 		supportedPCIDeviceMap := make(map[string]string)
 		for _, pciDev := range hostDevs.PciHostDevices {
diff --git a/pkg/virt-handler/device-manager/device_controller_test.go b/pkg/virt-handler/device-manager/device_controller_test.go
index ff10794a1..2cc22646f 100644
--- a/pkg/virt-handler/device-manager/device_controller_test.go
+++ b/pkg/virt-handler/device-manager/device_controller_test.go
@@ -54,6 +54,8 @@ var _ = Describe("Device Controller", func() {
 	var workDir string
 	var err error
 	var host string
+	var maxDevices int
+	var permissions string
 	var stop chan struct{}
 	var fakeConfigMap *virtconfig.ClusterConfig
 	var mockPCI *MockDeviceHandler
@@ -89,6 +91,8 @@ var _ = Describe("Device Controller", func() {
 		Expect(err).ToNot(HaveOccurred())
 
 		host = "master"
+		maxDevices = 100
+		permissions = "rw"
 		stop = make(chan struct{})
 	})
 
@@ -99,7 +103,7 @@ var _ = Describe("Device Controller", func() {
 	Context("Basic Tests", func() {
 		It("Should indicate if node has device", func() {
 			var noDevices []Device
-			deviceController := NewDeviceController(host, noDevices, fakeConfigMap, clientTest.CoreV1())
+			deviceController := NewDeviceController(host, maxDevices, permissions, noDevices, fakeConfigMap, clientTest.CoreV1())
 			devicePath := path.Join(workDir, "fake-device")
 			res := deviceController.NodeHasDevice(devicePath)
 			Expect(res).To(BeFalse())
@@ -136,7 +140,7 @@ var _ = Describe("Device Controller", func() {
 
 		It("should start the device plugin immediately without delays", func() {
 			initialDevices := []Device{plugin2}
-			deviceController := NewDeviceController(host, initialDevices, fakeConfigMap, clientTest.CoreV1())
+			deviceController := NewDeviceController(host, maxDevices, permissions, initialDevices, fakeConfigMap, clientTest.CoreV1())
 			deviceController.backoff = []time.Duration{10 * time.Millisecond, 10 * time.Second}
 
 			go deviceController.Run(stop)
@@ -151,7 +155,7 @@ var _ = Describe("Device Controller", func() {
 			plugin2.Error = fmt.Errorf("failing")
 			initialDevices := []Device{plugin2}
 
-			deviceController := NewDeviceController(host, initialDevices, fakeConfigMap, clientTest.CoreV1())
+			deviceController := NewDeviceController(host, maxDevices, permissions, initialDevices, fakeConfigMap, clientTest.CoreV1())
 			deviceController.backoff = []time.Duration{10 * time.Millisecond, 300 * time.Millisecond}
 
 			go deviceController.Run(stop)
@@ -163,7 +167,7 @@ var _ = Describe("Device Controller", func() {
 
 		It("Should not block on other plugins", func() {
 			initialDevices := []Device{plugin1, plugin2}
-			deviceController := NewDeviceController(host, initialDevices, fakeConfigMap, clientTest.CoreV1())
+			deviceController := NewDeviceController(host, maxDevices, permissions, initialDevices, fakeConfigMap, clientTest.CoreV1())
 
 			go deviceController.Run(stop)
 
@@ -184,7 +188,7 @@ var _ = Describe("Device Controller", func() {
 			Expect(emptyConfigMap.GetPermittedHostDevices()).To(BeNil())
 
 			initialDevices := []Device{plugin1, plugin2}
-			deviceController := NewDeviceController(host, initialDevices, emptyConfigMap, clientTest.CoreV1())
+			deviceController := NewDeviceController(host, maxDevices, permissions, initialDevices, emptyConfigMap, clientTest.CoreV1())
 
 			go deviceController.Run(stop)
 
diff --git a/pkg/virt-handler/device-manager/mediated_device_test.go b/pkg/virt-handler/device-manager/mediated_device_test.go
index 41e9b0736..7e2350283 100644
--- a/pkg/virt-handler/device-manager/mediated_device_test.go
+++ b/pkg/virt-handler/device-manager/mediated_device_test.go
@@ -184,7 +184,7 @@ var _ = Describe("Mediated Device", func() {
 
 			By("creating an empty device controller")
 			var noDevices []Device
-			deviceController := NewDeviceController("master", noDevices, fakeClusterConfig, clientTest.CoreV1())
+			deviceController := NewDeviceController("master", 100, "rw", noDevices, fakeClusterConfig, clientTest.CoreV1())
 
 			By("adding a host device to the cluster config")
 			kvConfig := kv.DeepCopy()
diff --git a/pkg/virt-handler/device-manager/mediated_devices_types_test.go b/pkg/virt-handler/device-manager/mediated_devices_types_test.go
index d0ab7adc4..e7ceff7a8 100644
--- a/pkg/virt-handler/device-manager/mediated_devices_types_test.go
+++ b/pkg/virt-handler/device-manager/mediated_devices_types_test.go
@@ -399,7 +399,7 @@ var _ = Describe("Mediated Devices Types configuration", func() {
 
 			By("creating an empty device controller")
 			var noDevices []Device
-			deviceController := NewDeviceController("master", noDevices, fakeClusterConfig, clientTest.CoreV1())
+			deviceController := NewDeviceController("master", 100, "rw", noDevices, fakeClusterConfig, clientTest.CoreV1())
 			deviceController.refreshMediatedDevicesTypes()
 			By("creating the desired mdev types")
 			desiredDevicesToConfigure := make(map[string]struct{})
diff --git a/pkg/virt-handler/device-manager/pci_device_test.go b/pkg/virt-handler/device-manager/pci_device_test.go
index 5c135ea32..217f69de1 100644
--- a/pkg/virt-handler/device-manager/pci_device_test.go
+++ b/pkg/virt-handler/device-manager/pci_device_test.go
@@ -129,7 +129,7 @@ pciHostDevices:
 
 		By("creating an empty device controller")
 		var noDevices []Device
-		deviceController := NewDeviceController("master", noDevices, fakeClusterConfig, clientTest.CoreV1())
+		deviceController := NewDeviceController("master", 100, "rw", noDevices, fakeClusterConfig, clientTest.CoreV1())
 
 		By("adding a host device to the cluster config")
 		kvConfig := kv.DeepCopy()
diff --git a/pkg/virt-handler/vm.go b/pkg/virt-handler/vm.go
index 425cd5955..06b9d4383 100644
--- a/pkg/virt-handler/vm.go
+++ b/pkg/virt-handler/vm.go
@@ -252,7 +252,13 @@ func NewController(
 		permissions = "rwm"
 	}
 
-	c.deviceManagerController = device_manager.NewDeviceController(c.host, device_manager.PermanentHostDevicePlugins(maxDevices, permissions), clusterConfig, clientset.CoreV1())
+	c.deviceManagerController = device_manager.NewDeviceController(
+		c.host,
+		maxDevices,
+		permissions,
+		device_manager.PermanentHostDevicePlugins(maxDevices, permissions),
+		clusterConfig,
+		clientset.CoreV1())
 	c.heartBeat = heartbeat.NewHeartBeat(clientset.CoreV1(), c.deviceManagerController, clusterConfig, host)
 
 	return c
-- 
2.39.1

From 806081dd1a12f4119f3fb9b4503b228507375378 Mon Sep 17 00:00:00 2001
From: Vasiliy Ulyanov <vulyanov@suse.de>
Date: Fri, 15 Jul 2022 10:44:41 +0200
Subject: [PATCH 2/2] Add functional test covering SEV device management

The test verifies that the SEV device is not advertised in the node's
capacity when the corresponding feature gate is disabled. In order to
successfully execute the test without SEV-capable hardware, a fake
device file is created, so it can be picked by the device manager.

Note: the test is labeled as serial becaue it alters the global kubevirt
configuration.

Signed-off-by: Vasiliy Ulyanov <vulyanov@suse.de>
---
 tests/launchsecurity/BUILD.bazel |   2 +
 tests/launchsecurity/sev.go      | 107 +++++++++++++++++++++++++------
 2 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/tests/launchsecurity/BUILD.bazel b/tests/launchsecurity/BUILD.bazel
index 850d3d653..63cae515e 100644
--- a/tests/launchsecurity/BUILD.bazel
+++ b/tests/launchsecurity/BUILD.bazel
@@ -7,6 +7,7 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/virt-config:go_default_library",
+        "//staging/src/kubevirt.io/client-go/kubecli:go_default_library",
         "//tests:go_default_library",
         "//tests/console:go_default_library",
         "//tests/framework/checks:go_default_library",
@@ -14,5 +15,6 @@ go_library(
         "//vendor/github.com/google/goexpect:go_default_library",
         "//vendor/github.com/onsi/ginkgo/v2:go_default_library",
         "//vendor/github.com/onsi/gomega:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
     ],
 )
diff --git a/tests/launchsecurity/sev.go b/tests/launchsecurity/sev.go
index 0bba648a4..a80c0f101 100644
--- a/tests/launchsecurity/sev.go
+++ b/tests/launchsecurity/sev.go
@@ -1,10 +1,18 @@
 package launchsecurity
 
 import (
+	"context"
+	"fmt"
+	"time"
+
 	expect "github.com/google/goexpect"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 
+	k8smetav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"kubevirt.io/client-go/kubecli"
+
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 	"kubevirt.io/kubevirt/tests"
 	"kubevirt.io/kubevirt/tests/console"
@@ -19,23 +27,86 @@ var _ = Describe("[sig-compute]AMD Secure Encrypted Virtualization (SEV)", func(
 		tests.BeforeTestCleanup()
 	})
 
-	It("should start a SEV VM", func() {
-		const secureBoot = false
-		vmi := libvmi.NewFedora(libvmi.WithUefi(secureBoot), libvmi.WithSEV())
-		vmi = tests.RunVMIAndExpectLaunch(vmi, 240)
-
-		By("Expecting the VirtualMachineInstance console")
-		Expect(console.LoginToFedora(vmi)).To(Succeed())
-
-		By("Verifying that SEV is enabled in the guest")
-		err := console.SafeExpectBatch(vmi, []expect.Batcher{
-			&expect.BSnd{S: "\n"},
-			&expect.BExp{R: console.PromptExpression},
-			&expect.BSnd{S: "dmesg | grep --color=never SEV\n"},
-			&expect.BExp{R: "AMD Memory Encryption Features active: SEV"},
-			&expect.BSnd{S: "\n"},
-			&expect.BExp{R: console.PromptExpression},
-		}, 30)
-		Expect(err).ToNot(HaveOccurred())
+	Context("[Serial]device management", func() {
+		var (
+			virtClient      kubecli.KubevirtClient
+			nodeName        string
+			isDevicePresent bool
+			err             error
+		)
+
+		BeforeEach(func() {
+			virtClient, err = kubecli.GetKubevirtClient()
+			Expect(err).ToNot(HaveOccurred())
+
+			nodeName = tests.NodeNameWithHandler()
+			Expect(nodeName).ToNot(BeEmpty())
+
+			checkCmd := []string{"ls", "/dev/sev"}
+			_, err = tests.ExecuteCommandInVirtHandlerPod(nodeName, checkCmd)
+			isDevicePresent = (err == nil)
+
+			if !isDevicePresent {
+				// Create a fake SEV device
+				mknodCmd := []string{"mknod", "/dev/sev", "c", "10", "124"}
+				_, err = tests.ExecuteCommandInVirtHandlerPod(nodeName, mknodCmd)
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			Eventually(func() bool {
+				node, err := virtClient.CoreV1().Nodes().Get(context.Background(), nodeName, k8smetav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				val, ok := node.Status.Capacity["devices.kubevirt.io/sev"]
+				return ok && !val.IsZero()
+			}, 30*time.Second, 1*time.Second).Should(BeTrue(), "SEV capacity should not be zero")
+		})
+
+		AfterEach(func() {
+			if !isDevicePresent {
+				// Remove the fake SEV device
+				rmCmd := []string{"rm", "-f", "/dev/sev"}
+				_, err = tests.ExecuteCommandInVirtHandlerPod(nodeName, rmCmd)
+				Expect(err).ToNot(HaveOccurred())
+			}
+
+			tests.EnableFeatureGate(virtconfig.WorkloadEncryptionSEV)
+		})
+
+		It("should reset SEV capacity when the feature gate is disabled", func() {
+			By(fmt.Sprintf("Disabling %s feature gate", virtconfig.WorkloadEncryptionSEV))
+			tests.DisableFeatureGate(virtconfig.WorkloadEncryptionSEV)
+			Eventually(func() bool {
+				node, err := virtClient.CoreV1().Nodes().Get(context.Background(), nodeName, k8smetav1.GetOptions{})
+				Expect(err).ToNot(HaveOccurred())
+				val, ok := node.Status.Capacity["devices.kubevirt.io/sev"]
+				return !ok || val.IsZero()
+			}, 30*time.Second, 1*time.Second).Should(BeTrue(), "SEV capacity should be zero")
+		})
+	})
+
+	Context("lifecycle", func() {
+		BeforeEach(func() {
+			checks.SkipTestIfNotSEVCapable()
+		})
+
+		It("should start a SEV VM", func() {
+			const secureBoot = false
+			vmi := libvmi.NewFedora(libvmi.WithUefi(secureBoot), libvmi.WithSEV())
+			vmi = tests.RunVMIAndExpectLaunch(vmi, 240)
+
+			By("Expecting the VirtualMachineInstance console")
+			Expect(console.LoginToFedora(vmi)).To(Succeed())
+
+			By("Verifying that SEV is enabled in the guest")
+			err := console.SafeExpectBatch(vmi, []expect.Batcher{
+				&expect.BSnd{S: "\n"},
+				&expect.BExp{R: console.PromptExpression},
+				&expect.BSnd{S: "dmesg | grep --color=never SEV\n"},
+				&expect.BExp{R: "AMD Memory Encryption Features active: SEV"},
+				&expect.BSnd{S: "\n"},
+				&expect.BExp{R: console.PromptExpression},
+			}, 30)
+			Expect(err).ToNot(HaveOccurred())
+		})
 	})
 })
-- 
2.39.1

Places

File 0009-Respect-the-feature-gate-when-exposing-SEV-device.patch of Package kubevirt (Revision 1de9bd49f2f0a02a72d854e59aa1ab66)

Places