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 authored and fidencio committed Nov 11, 2020
1 parent 9ac0e93 commit 4611567
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,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 @@ -229,7 +232,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
Original file line number Diff line number Diff line change
Expand Up @@ -96,10 +96,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 @@ -111,6 +111,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 @@ -175,7 +178,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 @@ -203,17 +206,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 @@ -228,9 +231,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 @@ -243,7 +246,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
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,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 @@ -541,6 +541,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 @@ -628,6 +629,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 @@ -713,6 +715,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 @@ -783,6 +786,7 @@ func newClhHypervisorConfig(h hypervisor) (vc.HypervisorConfig, error) {

return vc.HypervisorConfig{
HypervisorPath: hypervisor,
HypervisorPathList: h.HypervisorPathList,
KernelPath: kernel,
InitrdPath: initrd,
ImagePath: image,
Expand All @@ -801,6 +805,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
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,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 @@ -309,6 +312,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
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,7 @@ func (s *Sandbox) dumpConfig(ss *persistapi.SandboxState) {
FirmwarePath: sconfig.HypervisorConfig.FirmwarePath,
MachineAccelerators: sconfig.HypervisorConfig.MachineAccelerators,
HypervisorPath: sconfig.HypervisorConfig.HypervisorPath,
HypervisorPathList: sconfig.HypervisorConfig.HypervisorPathList,
HypervisorCtlPath: sconfig.HypervisorConfig.HypervisorCtlPath,
JailerPath: sconfig.HypervisorConfig.JailerPath,
BlockDeviceDriver: sconfig.HypervisorConfig.BlockDeviceDriver,
Expand All @@ -231,6 +232,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 @@ -511,6 +513,7 @@ func loadSandboxConfig(id string) (*SandboxConfig, error) {
FirmwarePath: hconf.FirmwarePath,
MachineAccelerators: hconf.MachineAccelerators,
HypervisorPath: hconf.HypervisorPath,
HypervisorPathList: hconf.HypervisorPathList,
HypervisorCtlPath: hconf.HypervisorCtlPath,
JailerPath: hconf.JailerPath,
BlockDeviceDriver: hconf.BlockDeviceDriver,
Expand All @@ -520,6 +523,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
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,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 @@ -91,6 +94,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
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"errors"
"fmt"
"path/filepath"
"regexp"
goruntime "runtime"
"strconv"
"strings"
Expand Down Expand Up @@ -202,6 +203,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 @@ -334,17 +344,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 @@ -372,7 +382,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 @@ -385,7 +395,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 @@ -649,7 +659,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 @@ -666,6 +676,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 @@ -698,7 +711,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 @@ -848,7 +861,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
Loading

0 comments on commit 4611567

Please sign in to comment.