config: Protect annotation for entropy_source

It would be undesirable to be given an annotation like "/dev/null".
Filter out bad annotation values.

Fixes: #1043

Suggested-by: James O. D. Hunt <james.o.hunt@intel.com>
Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>

docs/how-to/how-to-set-sandbox-config-kata.md

@@ -60,7 +60,7 @@ There are several kinds of Kata configurations and they are listed below.
 | `io.katacontainers.config.hypervisor.enable_swap` | `boolean` | enable swap of VM memory |
 | `io.katacontainers.config.hypervisor.enable_vhost_user_store` | `boolean` | enable vhost-user storage device (QEMU) |
 | `io.katacontainers.config.hypervisor.enable_virtio_mem` | `boolean` | enable virtio-mem (QEMU) |
-| `io.katacontainers.config.hypervisor.entropy_source` | string| the path to a host source of entropy (`/dev/random`, `/dev/urandom` or real hardware RNG device) |
+| `io.katacontainers.config.hypervisor.entropy_source` (R) | string| the path to a host source of entropy (`/dev/random`, `/dev/urandom` or real hardware RNG device) |
 | `io.katacontainers.config.hypervisor.file_mem_backend` (R) | string | file based memory backend root directory |
 | `io.katacontainers.config.hypervisor.firmware_hash` | string | container firmware SHA-512 hash value |
 | `io.katacontainers.config.hypervisor.firmware` | string | the guest firmware that will run the container VM |
@@ -197,6 +197,7 @@ the configuration entry:
 | Key | Config file entry | Comments |
 |-------| ----- | ----- |
 | `ctlpath`  | `valid_ctlpaths` | Valid paths for `acrnctl` binary |
+| `entropy_source` | `valid_entropy_sources` | Valid entropy sources, e.g. `/dev/random` |
 | `file_mem_backend`  | `valid_file_mem_backends` | Valid locations for the file-based memory backend root directory |
 | `jailer_path`  | `valid_jailer_paths`| Valid paths for the jailer constraining the container VM (Firecracker) |
 | `path`  | `valid_hypervisor_paths` | Valid hypervisors to run the container VM |

src/runtime/Makefile

@@ -166,6 +166,7 @@ DEFAULTEXPFEATURES := []
 
 #Default entropy source
 DEFENTROPYSOURCE := /dev/urandom
+DEFVALIDENTROPYSOURCES := [\"/dev/urandom\",\"/dev/random\",\"\"]
 
 DEFDISABLEBLOCK := false
 DEFSHAREDFS_QEMU_VIRTIOFS := virtio-fs
@@ -454,6 +455,7 @@ USER_VARS += DEFFILEMEMBACKEND
 USER_VARS += DEFVALIDFILEMEMBACKENDS
 USER_VARS += DEFMSIZE9P
 USER_VARS += DEFENTROPYSOURCE
+USER_VARS += DEFVALIDENTROPYSOURCES
 USER_VARS += DEFSANDBOXCGROUPONLY
 USER_VARS += DEFBINDMOUNTS
 USER_VARS += FEATURE_SELINUX

src/runtime/cli/config/configuration-fc.toml.in

@@ -161,23 +161,23 @@ block_device_driver = "@DEFBLOCKSTORAGEDRIVER_FC@"
 
 # This option changes the default hypervisor and kernel parameters
 # to enable debug output where available.
-# 
+#
 # Default false
 #enable_debug = true
 
 # Disable the customizations done in the runtime when it detects
 # that it is running on top a VMM. This will result in the runtime
 # behaving as it would when running on bare metal.
-# 
+#
 #disable_nesting_checks = true
 
-# This is the msize used for 9p shares. It is the number of bytes 
+# This is the msize used for 9p shares. It is the number of bytes
 # used for 9p packet payload.
 #msize_9p = @DEFMSIZE9P@
 
-# VFIO devices are hotplugged on a bridge by default. 
+# VFIO devices are hotplugged on a bridge by default.
 # Enable hotplugging on root bus. This may be required for devices with
-# a large PCI bar, as this is a current limitation with hotplugging on 
+# a large PCI bar, as this is a current limitation with hotplugging on
 # a bridge. This value is valid for "pc" machine type.
 # Default false
 #hotplug_vfio_on_root_bus = true
@@ -194,6 +194,11 @@ block_device_driver = "@DEFBLOCKSTORAGEDRIVER_FC@"
 # all practical purposes.
 #entropy_source= "@DEFENTROPYSOURCE@"
 
+# List of valid annotations values for entropy_source
+# The default if not set is empty (all annotations rejected.)
+# Your distribution recommends: @DEFVALIDENTROPYSOURCES@
+valid_entropy_sources = @DEFVALIDENTROPYSOURCES@
+
 # Path to OCI hook binaries in the *guest rootfs*.
 # This does not affect host-side hooks which must instead be added to
 # the OCI spec passed to the runtime.

src/runtime/cli/config/configuration-qemu.toml.in

@@ -296,6 +296,11 @@ pflashes = []
 # all practical purposes.
 #entropy_source= "@DEFENTROPYSOURCE@"
 
+# List of valid annotations values for entropy_source
+# The default if not set is empty (all annotations rejected.)
+# Your distribution recommends: @DEFVALIDENTROPYSOURCES@
+valid_entropy_sources = @DEFVALIDENTROPYSOURCES@
+
 # Path to OCI hook binaries in the *guest rootfs*.
 # This does not affect host-side hooks which must instead be added to
 # the OCI spec passed to the runtime.

src/runtime/pkg/katautils/config.go

@@ -99,6 +99,7 @@ type hypervisor struct {
 	PFlashList              []string `toml:"pflashes"`
 	VhostUserStorePathList  []string `toml:"valid_vhost_user_store_paths"`
 	FileBackedMemRootList   []string `toml:"valid_file_mem_backends"`
+	EntropySourceList       []string `toml:"valid_entropy_sources"`
 	EnableAnnotations       []string `toml:"enable_annotations"`
 	RxRateLimiterMaxRate    uint64   `toml:"rx_rate_limiter_max_rate"`
 	TxRateLimiterMaxRate    uint64   `toml:"tx_rate_limiter_max_rate"`
@@ -557,6 +558,7 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		MemorySize:            h.defaultMemSz(),
 		MemSlots:              h.defaultMemSlots(),
 		EntropySource:         h.GetEntropySource(),
+		EntropySourceList:     h.EntropySourceList,
 		DefaultBridges:        h.defaultBridges(),
 		DisableBlockDeviceUse: h.DisableBlockDeviceUse,
 		HugePages:             h.HugePages,
@@ -663,6 +665,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		MemOffset:               h.defaultMemOffset(),
 		VirtioMem:               h.VirtioMem,
 		EntropySource:           h.GetEntropySource(),
+		EntropySourceList:       h.EntropySourceList,
 		DefaultBridges:          h.defaultBridges(),
 		DisableBlockDeviceUse:   h.DisableBlockDeviceUse,
 		SharedFS:                sharedFS,
@@ -754,6 +757,7 @@ func newAcrnHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		MemorySize:            h.defaultMemSz(),
 		MemSlots:              h.defaultMemSlots(),
 		EntropySource:         h.GetEntropySource(),
+		EntropySourceList:     h.EntropySourceList,
 		DefaultBridges:        h.defaultBridges(),
 		HugePages:             h.HugePages,
 		Mlock:                 !h.Swap,
@@ -830,6 +834,7 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
 		MemOffset:               h.defaultMemOffset(),
 		VirtioMem:               h.VirtioMem,
 		EntropySource:           h.GetEntropySource(),
+		EntropySourceList:       h.EntropySourceList,
 		DefaultBridges:          h.defaultBridges(),
 		DisableBlockDeviceUse:   h.DisableBlockDeviceUse,
 		SharedFS:                sharedFS,

src/runtime/virtcontainers/hypervisor.go

@@ -310,6 +310,9 @@ type HypervisorConfig struct {
 	// entropy (/dev/random, /dev/urandom or real hardware RNG device)
 	EntropySource string
 
+	// EntropySourceList is the list of valid entropy sources
+	EntropySourceList []string
+
 	// Shared file system type:
 	//   - virtio-9p (default)
 	//   - virtio-fs

src/runtime/virtcontainers/persist.go

@@ -222,6 +222,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
 		MemoryPath:              sconfig.HypervisorConfig.MemoryPath,
 		DevicesStatePath:        sconfig.HypervisorConfig.DevicesStatePath,
 		EntropySource:           sconfig.HypervisorConfig.EntropySource,
+		EntropySourceList:       sconfig.HypervisorConfig.EntropySourceList,
 		SharedFS:                sconfig.HypervisorConfig.SharedFS,
 		VirtioFSDaemon:          sconfig.HypervisorConfig.VirtioFSDaemon,
 		VirtioFSDaemonList:      sconfig.HypervisorConfig.VirtioFSDaemonList,
@@ -491,6 +492,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
 		MemoryPath:              hconf.MemoryPath,
 		DevicesStatePath:        hconf.DevicesStatePath,
 		EntropySource:           hconf.EntropySource,
+		EntropySourceList:       hconf.EntropySourceList,
 		SharedFS:                hconf.SharedFS,
 		VirtioFSDaemon:          hconf.VirtioFSDaemon,
 		VirtioFSDaemonList:      hconf.VirtioFSDaemonList,

src/runtime/virtcontainers/persist/api/config.go

@@ -96,6 +96,9 @@ type HypervisorConfig struct {
 	// entropy (/dev/random, /dev/urandom or real hardware RNG device)
 	EntropySource string
 
+	// EntropySourceList is the list of valid entropy sources
+	EntropySourceList []string
+
 	// Shared file system type:
 	//   - virtio-9p (default)
 	//   - virtio-fs

src/runtime/virtcontainers/pkg/oci/utils.go

@@ -489,6 +489,9 @@ func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig,
 	}
 
 	if value, ok := ocispec.Annotations[vcAnnotations.EntropySource]; ok {
+		if !checkPathIsInGlobs(runtime.HypervisorConfig.EntropySourceList, value) {
+			return fmt.Errorf("entropy source %v required from annotation is not valid", value)
+		}
 		if value != "" {
 			config.HypervisorConfig.EntropySource = value
 		}

src/runtime/virtcontainers/pkg/oci/utils_test.go

@@ -858,7 +858,6 @@ func TestAddHypervisorAnnotations(t *testing.T) {
 	ocispec.Annotations[vcAnnotations.DisableImageNvdimm] = "true"
 	ocispec.Annotations[vcAnnotations.HotplugVFIOOnRootBus] = "true"
 	ocispec.Annotations[vcAnnotations.PCIeRootPort] = "2"
-	ocispec.Annotations[vcAnnotations.EntropySource] = "/dev/urandom"
 	ocispec.Annotations[vcAnnotations.IOMMUPlatform] = "true"
 	ocispec.Annotations[vcAnnotations.SGXEPC] = "64Mi"
 	// 10Mbit
@@ -895,7 +894,6 @@ func TestAddHypervisorAnnotations(t *testing.T) {
 	assert.Equal(config.HypervisorConfig.DisableImageNvdimm, true)
 	assert.Equal(config.HypervisorConfig.HotplugVFIOOnRootBus, true)
 	assert.Equal(config.HypervisorConfig.PCIeRootPort, uint32(2))
-	assert.Equal(config.HypervisorConfig.EntropySource, "/dev/urandom")
 	assert.Equal(config.HypervisorConfig.IOMMUPlatform, true)
 	assert.Equal(config.HypervisorConfig.SGXEPCSize, int64(67108864))
 	assert.Equal(config.HypervisorConfig.RxRateLimiterMaxRate, uint64(10000000))
@@ -945,22 +943,27 @@ func TestAddProtectedHypervisorAnnotations(t *testing.T) {
 
 	ocispec.Annotations[vcAnnotations.FileBackedMemRootDir] = "/dev/shm"
 	ocispec.Annotations[vcAnnotations.VirtioFSDaemon] = "/bin/false"
+	ocispec.Annotations[vcAnnotations.EntropySource] = "/dev/urandom"
 
 	config.HypervisorConfig.FileBackedMemRootDir = "do-not-touch"
 	config.HypervisorConfig.VirtioFSDaemon = "dangerous-daemon"
+	config.HypervisorConfig.EntropySource = "truly-random"
 
 	err = addAnnotations(ocispec, &config, runtimeConfig)
 	assert.Error(err)
 	assert.Equal(config.HypervisorConfig.FileBackedMemRootDir, "do-not-touch")
 	assert.Equal(config.HypervisorConfig.VirtioFSDaemon, "dangerous-daemon")
+	assert.Equal(config.HypervisorConfig.EntropySource, "truly-random")
 
 	// Now enable them and check again
 	runtimeConfig.HypervisorConfig.FileBackedMemRootList = []string{"/dev/*m"}
 	runtimeConfig.HypervisorConfig.VirtioFSDaemonList = []string{"/bin/*ls*"}
+	runtimeConfig.HypervisorConfig.EntropySourceList = []string{"/dev/*random*"}
 	err = addAnnotations(ocispec, &config, runtimeConfig)
 	assert.NoError(err)
 	assert.Equal(config.HypervisorConfig.FileBackedMemRootDir, "/dev/shm")
 	assert.Equal(config.HypervisorConfig.VirtioFSDaemon, "/bin/false")
+	assert.Equal(config.HypervisorConfig.EntropySource, "/dev/urandom")
 
 	// In case an absurd large value is provided, the config value if not over-ridden
 	ocispec.Annotations[vcAnnotations.DefaultVCPUs] = "655536"