Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
home:dirkmueller:acdc:sp5-rebuild
kubevirt.31631
0009-Respect-the-feature-gate-when-exposing-SEV...
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File 0009-Respect-the-feature-gate-when-exposing-SEV-device.patch of Package kubevirt.31631
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
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor