Skip to content
This repository has been archived by the owner on May 12, 2021. It is now read-only.

Commit

Permalink
config: Protect virtio_fs_daemon annotation
Browse files Browse the repository at this point in the history
Sending the virtio_fs_daemon annotation can be used to execute
arbitrary code on the host. In order to prevent this, restrict the
values of the annotation to a list provided by the configuration
file.

Fixes: #3004

Signed-off-by: Christophe de Dinechin <dinechin@redhat.com>
  • Loading branch information
c3d committed Nov 10, 2020
1 parent 11e737d commit 22e89f6
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 29 deletions.
3 changes: 3 additions & 0 deletions cli/config/configuration-clh.toml.in
Expand Up @@ -62,6 +62,9 @@ default_memory = @DEFMEMSZ@
# Path to vhost-user-fs daemon.
virtio_fs_daemon = "@DEFVIRTIOFSDAEMON@"

# List of valid annotations values for the virtiofs daemon (default: empty)
# virtio_fs_daemon_list = [ "/opt/kata/bin/virtiofsd", "/usr/.*/virtiofsd" ]

# Default size of DAX cache in MiB
virtio_fs_cache_size = @DEFVIRTIOFSCACHESIZE@

Expand Down
5 changes: 4 additions & 1 deletion cli/config/configuration-qemu-virtiofs.toml.in
Expand Up @@ -110,6 +110,9 @@ shared_fs = "@DEFSHAREDFS_QEMU_VIRTIOFS@"
# Path to vhost-user-fs daemon.
virtio_fs_daemon = "@DEFVIRTIOFSDAEMON@"

# List of valid annotations values for the virtiofs daemon (default: empty)
# virtio_fs_daemon_list = [ "/opt/kata/bin/virtiofsd", "/usr/.*/virtiofsd" ]

# Default size of DAX cache in MiB
virtio_fs_cache_size = @DEFVIRTIOFSCACHESIZE@

Expand Down Expand Up @@ -245,7 +248,7 @@ vhost_user_store_path = "@DEFVHOSTUSERSTOREPATH@"
#hotplug_vfio_on_root_bus = true

# If vhost-net backend for virtio-net is not desired, set to true. Default is false, which trades off
# security (vhost-net runs ring0) for network I/O performance.
# security (vhost-net runs ring0) for network I/O performance.
#disable_vhost_net = true

#
Expand Down
23 changes: 13 additions & 10 deletions cli/config/configuration-qemu.toml.in
Expand Up @@ -102,10 +102,10 @@ default_memory = @DEFMEMSZ@
#enable_virtio_mem = true

# Disable block device from being used for a container's rootfs.
# In case of a storage driver like devicemapper where a container's
# In case of a storage driver like devicemapper where a container's
# root file system is backed by a block device, the block device is passed
# directly to the hypervisor for performance reasons.
# This flag prevents the block device from being passed to the hypervisor,
# directly to the hypervisor for performance reasons.
# This flag prevents the block device from being passed to the hypervisor,
# 9pfs is used instead to pass the rootfs.
disable_block_device_use = @DEFDISABLEBLOCK@

Expand All @@ -117,6 +117,9 @@ shared_fs = "@DEFSHAREDFS@"
# Path to vhost-user-fs daemon.
virtio_fs_daemon = "@DEFVIRTIOFSDAEMON@"

# List of valid annotations values for the virtiofs daemon (default: empty)
# virtio_fs_daemon_list = [ "/opt/kata/bin/virtiofsd", "/usr/.*/virtiofsd" ]

# Default size of DAX cache in MiB
virtio_fs_cache_size = @DEFVIRTIOFSCACHESIZE@

Expand Down Expand Up @@ -181,7 +184,7 @@ enable_iothreads = @DEFENABLEIOTHREADS@
# Enabling this will result in the VM memory
# being allocated using huge pages.
# This is useful when you want to use vhost-user network
# stacks within the container. This will automatically
# stacks within the container. This will automatically
# result in memory pre allocation
#enable_hugepages = true

Expand Down Expand Up @@ -219,17 +222,17 @@ vhost_user_store_path = "@DEFVHOSTUSERSTOREPATH@"
# This option changes the default hypervisor and kernel parameters
# to enable debug output where available. This extra output is added
# to the proxy logs, but only when proxy debug is also enabled.
#
#
# 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@

Expand All @@ -244,9 +247,9 @@ vhost_user_store_path = "@DEFVHOSTUSERSTOREPATH@"
# Default is false
#disable_image_nvdimm = true

# 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
Expand All @@ -259,7 +262,7 @@ vhost_user_store_path = "@DEFVHOSTUSERSTOREPATH@"
#pcie_root_port = 2

# If vhost-net backend for virtio-net is not desired, set to true. Default is false, which trades off
# security (vhost-net runs ring0) for network I/O performance.
# security (vhost-net runs ring0) for network I/O performance.
#disable_vhost_net = true

#
Expand Down
7 changes: 6 additions & 1 deletion pkg/katautils/config.go
Expand Up @@ -86,7 +86,7 @@ type factory struct {

type hypervisor struct {
Path string `toml:"path"`
PathList []string `toml:"path_list"`
HypervisorPathList []string `toml:"path_list"`
JailerPath string `toml:"jailer_path"`
JailerPathList []string `toml:"jailer_path_list"`
Kernel string `toml:"kernel"`
Expand Down Expand Up @@ -566,6 +566,7 @@ func newFirecrackerHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {

return vc.HypervisorConfig{
HypervisorPath: hypervisor,
HypervisorPathList: h.HypervisorPathList,
JailerPath: jailer,
KernelPath: kernel,
InitrdPath: initrd,
Expand Down Expand Up @@ -662,6 +663,7 @@ func newQemuHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {

return vc.HypervisorConfig{
HypervisorPath: hypervisor,
HypervisorPathList: h.HypervisorPathList,
KernelPath: kernel,
InitrdPath: initrd,
ImagePath: image,
Expand Down Expand Up @@ -750,6 +752,7 @@ func newAcrnHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {

return vc.HypervisorConfig{
HypervisorPath: hypervisor,
HypervisorPathList: h.HypervisorPathList,
KernelPath: kernel,
ImagePath: image,
HypervisorCtlPath: hypervisorctl,
Expand Down Expand Up @@ -820,6 +823,7 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {

return vc.HypervisorConfig{
HypervisorPath: hypervisor,
HypervisorPathList: h.HypervisorPathList,
KernelPath: kernel,
InitrdPath: initrd,
ImagePath: image,
Expand All @@ -838,6 +842,7 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {
DisableBlockDeviceUse: h.DisableBlockDeviceUse,
SharedFS: sharedFS,
VirtioFSDaemon: h.VirtioFSDaemon,
VirtioFSDaemonList: h.VirtioFSDaemonList,
VirtioFSCacheSize: h.VirtioFSCacheSize,
VirtioFSCache: h.VirtioFSCache,
MemPrealloc: h.MemPrealloc,
Expand Down
6 changes: 6 additions & 0 deletions virtcontainers/hypervisor.go
Expand Up @@ -278,6 +278,9 @@ type HypervisorConfig struct {
// HypervisorPath is the hypervisor executable host path.
HypervisorPath string

// HypervisorPathList is the list of hypervisor paths names allowed in annotations
HypervisorPathList []string

// HypervisorCtlPath is the hypervisor ctl executable host path.
HypervisorCtlPath string

Expand Down Expand Up @@ -312,6 +315,9 @@ type HypervisorConfig struct {
// VirtioFSDaemon is the virtio-fs vhost-user daemon path
VirtioFSDaemon string

// VirtioFSDaemonList is the list of valid virtiofs names for annotations
VirtioFSDaemonList []string

// VirtioFSCache cache mode for fs version cache or "none"
VirtioFSCache string

Expand Down
4 changes: 4 additions & 0 deletions virtcontainers/persist.go
Expand Up @@ -224,6 +224,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
MachineAccelerators: sconfig.HypervisorConfig.MachineAccelerators,
CPUFeatures: sconfig.HypervisorConfig.CPUFeatures,
HypervisorPath: sconfig.HypervisorConfig.HypervisorPath,
HypervisorPathList: sconfig.HypervisorConfig.HypervisorPathList,
HypervisorCtlPath: sconfig.HypervisorConfig.HypervisorCtlPath,
JailerPath: sconfig.HypervisorConfig.JailerPath,
BlockDeviceDriver: sconfig.HypervisorConfig.BlockDeviceDriver,
Expand All @@ -233,6 +234,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
EntropySource: sconfig.HypervisorConfig.EntropySource,
SharedFS: sconfig.HypervisorConfig.SharedFS,
VirtioFSDaemon: sconfig.HypervisorConfig.VirtioFSDaemon,
VirtioFSDaemonList: sconfig.HypervisorConfig.VirtioFSDaemonList,
VirtioFSCache: sconfig.HypervisorConfig.VirtioFSCache,
VirtioFSExtraArgs: sconfig.HypervisorConfig.VirtioFSExtraArgs[:],
BlockDeviceCacheSet: sconfig.HypervisorConfig.BlockDeviceCacheSet,
Expand Down Expand Up @@ -515,6 +517,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
MachineAccelerators: hconf.MachineAccelerators,
CPUFeatures: hconf.CPUFeatures,
HypervisorPath: hconf.HypervisorPath,
HypervisorPathList: hconf.HypervisorPathList,
HypervisorCtlPath: hconf.HypervisorCtlPath,
JailerPath: hconf.JailerPath,
BlockDeviceDriver: hconf.BlockDeviceDriver,
Expand All @@ -524,6 +527,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
EntropySource: hconf.EntropySource,
SharedFS: hconf.SharedFS,
VirtioFSDaemon: hconf.VirtioFSDaemon,
VirtioFSDaemonList: hconf.VirtioFSDaemonList,
VirtioFSCache: hconf.VirtioFSCache,
VirtioFSExtraArgs: hconf.VirtioFSExtraArgs[:],
BlockDeviceCacheSet: hconf.BlockDeviceCacheSet,
Expand Down
6 changes: 6 additions & 0 deletions virtcontainers/persist/api/config.go
Expand Up @@ -60,6 +60,9 @@ type HypervisorConfig struct {
// HypervisorPath is the hypervisor executable host path.
HypervisorPath string

// HypervisorPathList is the list of hypervisor paths names allowed in annotations
HypervisorPathList []string

// HypervisorCtlPath is the hypervisor ctl executable host path.
HypervisorCtlPath string

Expand Down Expand Up @@ -94,6 +97,9 @@ type HypervisorConfig struct {
// VirtioFSDaemon is the virtio-fs vhost-user daemon path
VirtioFSDaemon string

// VirtioFSDaemonList is the list of valid virtiofs names for annotations
VirtioFSDaemonList []string

// VirtioFSCache cache mode for fs version cache or "none"
VirtioFSCache string

Expand Down
29 changes: 21 additions & 8 deletions virtcontainers/pkg/oci/utils.go
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"path/filepath"
"regexp"
goruntime "runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -205,6 +206,15 @@ func contains(s []string, e string) bool {
return false
}

func regexpContains(s []string, e string) bool {
for _, a := range s {
if matched, _ := regexp.MatchString(a, e); matched {
return true
}
}
return false
}

func newLinuxDeviceInfo(d specs.LinuxDevice) (*config.DeviceInfo, error) {
allowedDeviceTypes := []string{"c", "b", "u", "p"}

Expand Down Expand Up @@ -337,17 +347,17 @@ func SandboxID(spec specs.Spec) (string, error) {
return "", fmt.Errorf("Could not find sandbox ID")
}

func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error {
func addAnnotations(ocispec specs.Spec, config *vc.SandboxConfig, runtime RuntimeConfig) error {
err := addAssetAnnotations(ocispec, config)
if err != nil {
return err
}

if err := addHypervisorConfigOverrides(ocispec, config); err != nil {
if err := addHypervisorConfigOverrides(ocispec, config, runtime); err != nil {
return err
}

if err := addRuntimeConfigOverrides(ocispec, config); err != nil {
if err := addRuntimeConfigOverrides(ocispec, config, runtime); err != nil {
return err
}

Expand Down Expand Up @@ -375,7 +385,7 @@ func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) error {
return nil
}

func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig) error {
func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig, runtime RuntimeConfig) error {
if err := addHypervisorCPUOverrides(ocispec, config); err != nil {
return err
}
Expand All @@ -388,7 +398,7 @@ func addHypervisorConfigOverrides(ocispec specs.Spec, config *vc.SandboxConfig)
return err
}

if err := addHypervisporVirtioFsOverrides(ocispec, config); err != nil {
if err := addHypervisporVirtioFsOverrides(ocispec, config, runtime); err != nil {
return err
}

Expand Down Expand Up @@ -676,7 +686,7 @@ func addHypervisorBlockOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig)
return nil
}

func addHypervisporVirtioFsOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig) error {
func addHypervisporVirtioFsOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig, runtime RuntimeConfig) error {
if value, ok := ocispec.Annotations[vcAnnotations.SharedFS]; ok {
supportedSharedFS := []string{config.Virtio9P, config.VirtioFS}
valid := false
Expand All @@ -693,6 +703,9 @@ func addHypervisporVirtioFsOverrides(ocispec specs.Spec, sbConfig *vc.SandboxCon
}

if value, ok := ocispec.Annotations[vcAnnotations.VirtioFSDaemon]; ok {
if !regexpContains(runtime.HypervisorConfig.VirtioFSDaemonList, value) {
return fmt.Errorf("virtiofs daemon %v required from annotation is not valid", value)
}
sbConfig.HypervisorConfig.VirtioFSDaemon = value
}

Expand Down Expand Up @@ -725,7 +738,7 @@ func addHypervisporVirtioFsOverrides(ocispec specs.Spec, sbConfig *vc.SandboxCon
return nil
}

func addRuntimeConfigOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig) error {
func addRuntimeConfigOverrides(ocispec specs.Spec, sbConfig *vc.SandboxConfig, runtime RuntimeConfig) error {
if value, ok := ocispec.Annotations[vcAnnotations.DisableGuestSeccomp]; ok {
disableGuestSeccomp, err := strconv.ParseBool(value)
if err != nil {
Expand Down Expand Up @@ -877,7 +890,7 @@ func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, c
Experimental: runtime.Experimental,
}

if err := addAnnotations(ocispec, &sandboxConfig); err != nil {
if err := addAnnotations(ocispec, &sandboxConfig, runtime); err != nil {
return vc.SandboxConfig{}, err
}

Expand Down

0 comments on commit 22e89f6

Please sign in to comment.