File 0009-Respect-the-feature-gate-when-exposing-SEV-device.patch of Package kubevirt.29906
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
