From b5f2a1e8c4d445999b32b1ddb41d5ed0006c0034 Mon Sep 17 00:00:00 2001 From: Christophe de Dinechin Date: Fri, 15 May 2020 18:10:53 +0200 Subject: [PATCH] config: Protect ctlpath from annotation attack This also adds annotation for ctlpath which were not present before. It's better to implement the code consistenly right now to make sure that we don't end up with a leaky implementation tacked on later. Fixes: #901 Signed-off-by: Christophe de Dinechin --- .../cli/config/configuration-acrn.toml.in | 3 ++ src/runtime/pkg/katautils/config.go | 41 ++++++++++--------- src/runtime/virtcontainers/hypervisor.go | 3 ++ src/runtime/virtcontainers/persist.go | 2 + .../virtcontainers/persist/api/config.go | 4 ++ .../pkg/annotations/annotations.go | 3 ++ src/runtime/virtcontainers/pkg/oci/utils.go | 7 ++++ 7 files changed, 43 insertions(+), 20 deletions(-) diff --git a/src/runtime/cli/config/configuration-acrn.toml.in b/src/runtime/cli/config/configuration-acrn.toml.in index 7bc8d3172..f6e9b7849 100644 --- a/src/runtime/cli/config/configuration-acrn.toml.in +++ b/src/runtime/cli/config/configuration-acrn.toml.in @@ -20,6 +20,9 @@ image = "@IMAGEPATH@" # Each member of the list can be a regular expression # path_list = [ "@ACRNPATH@.*" ] +# List of valid annotations values for ctlpath (default: empty) +# ctlpath_list = [ "@ACRNCTLPATH@.*" ] + # Optional space-separated list of options to pass to the guest kernel. # For example, use `kernel_params = "vsyscall=emulate"` if you are having # trouble running pre-2.15 glibc. diff --git a/src/runtime/pkg/katautils/config.go b/src/runtime/pkg/katautils/config.go index 17f16fa92..adb398bd7 100644 --- a/src/runtime/pkg/katautils/config.go +++ b/src/runtime/pkg/katautils/config.go @@ -725,26 +725,27 @@ func newAcrnHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) { } return vc.HypervisorConfig{ - HypervisorPath: hypervisor, - HypervisorPathList: h.HypervisorPathList, - KernelPath: kernel, - ImagePath: image, - HypervisorCtlPath: hypervisorctl, - FirmwarePath: firmware, - KernelParams: vc.DeserializeParams(strings.Fields(kernelParams)), - NumVCPUs: h.defaultVCPUs(), - DefaultMaxVCPUs: h.defaultMaxVCPUs(), - MemorySize: h.defaultMemSz(), - MemSlots: h.defaultMemSlots(), - EntropySource: h.GetEntropySource(), - DefaultBridges: h.defaultBridges(), - HugePages: h.HugePages, - Mlock: !h.Swap, - Debug: h.Debug, - DisableNestingChecks: h.DisableNestingChecks, - BlockDeviceDriver: blockDriver, - DisableVhostNet: h.DisableVhostNet, - GuestHookPath: h.guestHookPath(), + HypervisorPath: hypervisor, + HypervisorPathList: h.HypervisorPathList, + KernelPath: kernel, + ImagePath: image, + HypervisorCtlPath: hypervisorctl, + HypervisorCtlPathList: h.CtlPathList, + FirmwarePath: firmware, + KernelParams: vc.DeserializeParams(strings.Fields(kernelParams)), + NumVCPUs: h.defaultVCPUs(), + DefaultMaxVCPUs: h.defaultMaxVCPUs(), + MemorySize: h.defaultMemSz(), + MemSlots: h.defaultMemSlots(), + EntropySource: h.GetEntropySource(), + DefaultBridges: h.defaultBridges(), + HugePages: h.HugePages, + Mlock: !h.Swap, + Debug: h.Debug, + DisableNestingChecks: h.DisableNestingChecks, + BlockDeviceDriver: blockDriver, + DisableVhostNet: h.DisableVhostNet, + GuestHookPath: h.guestHookPath(), }, nil } diff --git a/src/runtime/virtcontainers/hypervisor.go b/src/runtime/virtcontainers/hypervisor.go index 1c65c16d8..a50fd5f5a 100644 --- a/src/runtime/virtcontainers/hypervisor.go +++ b/src/runtime/virtcontainers/hypervisor.go @@ -278,6 +278,9 @@ type HypervisorConfig struct { // HypervisorPathList is the list of hypervisor paths names allowed in annotations HypervisorPathList []string + // HypervisorCtlPathList is the list of hypervisor control paths names allowed in annotations + HypervisorCtlPathList []string + // HypervisorCtlPath is the hypervisor ctl executable host path. HypervisorCtlPath string diff --git a/src/runtime/virtcontainers/persist.go b/src/runtime/virtcontainers/persist.go index c02b0868a..9e6789fe7 100644 --- a/src/runtime/virtcontainers/persist.go +++ b/src/runtime/virtcontainers/persist.go @@ -214,6 +214,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) { HypervisorPath: sconfig.HypervisorConfig.HypervisorPath, HypervisorPathList: sconfig.HypervisorConfig.HypervisorPathList, HypervisorCtlPath: sconfig.HypervisorConfig.HypervisorCtlPath, + HypervisorCtlPathList: sconfig.HypervisorConfig.HypervisorCtlPathList, JailerPath: sconfig.HypervisorConfig.JailerPath, JailerPathList: sconfig.HypervisorConfig.JailerPathList, BlockDeviceDriver: sconfig.HypervisorConfig.BlockDeviceDriver, @@ -478,6 +479,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) { HypervisorPath: hconf.HypervisorPath, HypervisorPathList: hconf.HypervisorPathList, HypervisorCtlPath: hconf.HypervisorCtlPath, + HypervisorCtlPathList: hconf.HypervisorCtlPathList, JailerPath: hconf.JailerPath, JailerPathList: hconf.JailerPathList, BlockDeviceDriver: hconf.BlockDeviceDriver, diff --git a/src/runtime/virtcontainers/persist/api/config.go b/src/runtime/virtcontainers/persist/api/config.go index 86b1f2565..4b09e2aca 100644 --- a/src/runtime/virtcontainers/persist/api/config.go +++ b/src/runtime/virtcontainers/persist/api/config.go @@ -66,6 +66,10 @@ type HypervisorConfig struct { // HypervisorCtlPath is the hypervisor ctl executable host path. HypervisorCtlPath string + // HypervisorCtlPathList is the list of hypervisor control paths names allowed in annotations + HypervisorCtlPathList []string + + // HypervisorCtlPath is the hypervisor ctl executable host path. // JailerPath is the jailer executable host path. JailerPath string diff --git a/src/runtime/virtcontainers/pkg/annotations/annotations.go b/src/runtime/virtcontainers/pkg/annotations/annotations.go index b9b3bf2fa..14d50a3c0 100644 --- a/src/runtime/virtcontainers/pkg/annotations/annotations.go +++ b/src/runtime/virtcontainers/pkg/annotations/annotations.go @@ -44,6 +44,9 @@ const ( // JailerPath is a sandbox annotation for passing a per container path pointing at the jailer that will constrain the container VM. JailerPath = kataAnnotHypervisorPrefix + "jailer_path" + // CtlPath is a sandbox annotation for passing a per container path pointing at the acrn ctl binary + CtlPath = kataAnnotHypervisorPrefix + "ctlpath" + // FirmwarePath is a sandbox annotation for passing a per container path pointing at the guest firmware that will run the container VM. FirmwarePath = kataAnnotHypervisorPrefix + "firmware" diff --git a/src/runtime/virtcontainers/pkg/oci/utils.go b/src/runtime/virtcontainers/pkg/oci/utils.go index 236ea47dc..c632a664d 100644 --- a/src/runtime/virtcontainers/pkg/oci/utils.go +++ b/src/runtime/virtcontainers/pkg/oci/utils.go @@ -404,6 +404,13 @@ func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig, config.HypervisorConfig.JailerPath = value } + if value, ok := ocispec.Annotations[vcAnnotations.CtlPath]; ok { + if !regexpContains(runtime.HypervisorConfig.HypervisorCtlPathList, value) { + return fmt.Errorf("hypervisor control %v required from annotation is not valid", value) + } + config.HypervisorConfig.HypervisorCtlPath = value + } + if value, ok := ocispec.Annotations[vcAnnotations.KernelParams]; ok { if value != "" { params := vc.DeserializeParams(strings.Fields(value))