From ff17c756d2cb777ce63af2c9aa5a3dd7dc5e2eba Mon Sep 17 00:00:00 2001 From: Jakob Naucke Date: Tue, 11 Jan 2022 19:09:56 +0100 Subject: [PATCH] runtime: Allow and require no initrd for SE Previously, it was not permitted to have neither an initrd nor an image. However, this is the exact config to use for Secure Execution, where the initrd is part of the image to be specified as `-kernel`. Require the configuration of no initrd for Secure Execution. Also - remove redundant code for image/initrd checking -- no need to check in `newQemuHypervisorConfig` (calling) when it is also checked in `getInitrdAndImage` (called) - use `QemuCCWVirtio` constant when possible Fixes: #3922 Signed-off-by: Jakob Naucke --- src/runtime/pkg/katautils/config.go | 20 ++++++------------- src/runtime/virtcontainers/hypervisor.go | 14 +++++++------ src/runtime/virtcontainers/hypervisor_test.go | 12 +++++++++++ src/runtime/virtcontainers/qemu.go | 2 +- 4 files changed, 27 insertions(+), 21 deletions(-) diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 741f326316..3525bc236e 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -469,11 +469,13 @@ func (h hypervisor) getInitrdAndImage() (initrd string, image string, err error) image, errImage := h.image() - if image != "" && initrd != "" { + if h.ConfidentialGuest && h.MachineType == vc.QemuCCWVirtio { + if image != "" || initrd != "" { + return "", "", errors.New("Neither the image nor initrd path may be set for Secure Execution") + } + } else if image != "" && initrd != "" { return "", "", errors.New("having both an image and an initrd defined in the configuration file is not supported") - } - - if errInitrd != nil && errImage != nil { + } else if errInitrd != nil && errImage != nil { return "", "", fmt.Errorf("Either initrd or image must be set to a valid path (initrd: %v) (image: %v)", errInitrd, errImage) } @@ -605,16 +607,6 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { return vc.HypervisorConfig{}, err } - if image != "" && initrd != "" { - return vc.HypervisorConfig{}, - errors.New("having both an image and an initrd defined in the configuration file is not supported") - } - - if image == "" && initrd == "" { - return vc.HypervisorConfig{}, - errors.New("either image or initrd must be defined in the configuration file") - } - firmware, err := h.firmware() if err != nil { return vc.HypervisorConfig{}, err diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index cebc515705..12725160b8 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -527,17 +527,19 @@ func (conf *HypervisorConfig) CheckTemplateConfig() error { } func (conf *HypervisorConfig) Valid() error { - // Kata specific checks. Should be done outside the hypervisor if conf.KernelPath == "" { return fmt.Errorf("Missing kernel path") } - if conf.ImagePath == "" && conf.InitrdPath == "" { + if conf.ConfidentialGuest && conf.HypervisorMachineType == QemuCCWVirtio { + if conf.ImagePath != "" || conf.InitrdPath != "" { + fmt.Println("yes, failing") + return fmt.Errorf("Neither the image or initrd path may be set for Secure Execution") + } + } else if conf.ImagePath == "" && conf.InitrdPath == "" { return fmt.Errorf("Missing image and initrd path") - } - - if conf.ImagePath != "" && conf.InitrdPath != "" { + } else if conf.ImagePath != "" && conf.InitrdPath != "" { return fmt.Errorf("Image and initrd path cannot be both set") } @@ -559,7 +561,7 @@ func (conf *HypervisorConfig) Valid() error { if conf.BlockDeviceDriver == "" { conf.BlockDeviceDriver = defaultBlockDriver - } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == "s390-ccw-virtio" { + } else if conf.BlockDeviceDriver == config.VirtioBlock && conf.HypervisorMachineType == QemuCCWVirtio { conf.BlockDeviceDriver = config.VirtioBlockCCW } diff --git a/src/runtime/virtcontainers/hypervisor_test.go b/src/runtime/virtcontainers/hypervisor_test.go index 86aefde697..ab312b0982 100644 --- a/src/runtime/virtcontainers/hypervisor_test.go +++ b/src/runtime/virtcontainers/hypervisor_test.go @@ -144,6 +144,18 @@ func TestHypervisorConfigBothInitrdAndImage(t *testing.T) { testHypervisorConfigValid(t, hypervisorConfig, false) } +func TestHypervisorConfigSecureExecution(t *testing.T) { + hypervisorConfig := &HypervisorConfig{ + KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), + InitrdPath: fmt.Sprintf("%s/%s", testDir, testInitrd), + ConfidentialGuest: true, + HypervisorMachineType: QemuCCWVirtio, + } + + // Secure Execution should only specify a kernel (encrypted image contains all components) + testHypervisorConfigValid(t, hypervisorConfig, false) +} + func TestHypervisorConfigValidTemplateConfig(t *testing.T) { hypervisorConfig := &HypervisorConfig{ KernelPath: fmt.Sprintf("%s/%s", testDir, testKernel), diff --git a/src/runtime/virtcontainers/qemu.go b/src/runtime/virtcontainers/qemu.go index 242b3645dd..b76ba3fe5e 100644 --- a/src/runtime/virtcontainers/qemu.go +++ b/src/runtime/virtcontainers/qemu.go @@ -1841,7 +1841,7 @@ func (q *qemu) hotplugAddCPUs(amount uint32) (uint32, error) { threadID := fmt.Sprintf("%d", hc.Properties.Thread) // If CPU type is IBM pSeries, Z or arm virt, we do not set socketID and threadID - if machine.Type == "pseries" || machine.Type == "s390-ccw-virtio" || machine.Type == "virt" { + if machine.Type == "pseries" || machine.Type == QemuCCWVirtio || machine.Type == "virt" { socketID = "" threadID = "" dieID = ""