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: #3004

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

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.

pkg/katautils/config.go

@@ -752,26 +752,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
 }
 

virtcontainers/hypervisor.go

@@ -281,6 +281,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
 

virtcontainers/persist.go

@@ -226,6 +226,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,
@@ -520,6 +521,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,

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
 

virtcontainers/pkg/annotations/annotations.go

@@ -47,6 +47,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"
 

virtcontainers/pkg/oci/utils.go

@@ -414,6 +414,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))