Skip to content

Commit

Permalink
Add default mount options to pass to drivers
Browse files Browse the repository at this point in the history
I believe we should be running container images mounted with nodev by default.
This would eliminate the disk of a device sneaking into the container without
being on the approved list.  This would give us the same or potentially additional
security over the device cgroup.

It would be nice if this could be passed in on an image by image basis.  So users
could also specify if they want nosuid images.

Signed-off-by: Daniel J Walsh <[email protected]>
  • Loading branch information
rhatdan committed May 30, 2018
1 parent 8a34ffd commit 8f977dd
Show file tree
Hide file tree
Showing 10 changed files with 51 additions and 20 deletions.
8 changes: 4 additions & 4 deletions docs/containers-storage.conf.5.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,6 @@ Specifies the min free space percent in a thin pool required for new device crea

Specifies extra mkfs arguments to be used when creating the base device.

**mountopt**=""

Specifies extra mount options used when mounting the thin devices.

**use_deferred_removal**=""

Marks devicemapper block device for deferred removal. If the device is in use when its driver attempts to remove it, the driver tells the kernel to remove the device as soon as possible. Note this does not free up the disk space, use deferred deletion to fully remove the thinpool. (default: true).
Expand All @@ -125,6 +121,10 @@ Marks thinpool device for deferred deletion. If the thinpool is in use when the

Specifies the maximum number of retries XFS should attempt to complete IO when ENOSPC (no space) error is returned by underlying storage device. (default: 0, which means to try continuously.)

**mountopt**=""

Default options to be used to mount container images. Suggested value "nodev".

# HISTORY
May 2017, Originally compiled by Dan Walsh <[email protected]>
Format copied from crio.conf man page created by Aleksa Sarai <[email protected]>
7 changes: 7 additions & 0 deletions drivers/aufs/aufs.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
return nil, errors.Wrapf(graphdriver.ErrIncompatibleFS, "AUFS is not supported over %q", backingFs)
}

for _, option := range options {
if strings.HasPrefix(option, "aufs.mountopt=") {
return nil, fmt.Errorf("aufs driver does not support mount options")
} else {
return nil, fmt.Errorf("option %s not supported", option)
}
}
paths := []string{
"mnt",
"diff",
Expand Down
7 changes: 5 additions & 2 deletions drivers/btrfs/btrfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,9 @@ func init() {
}

type btrfsOptions struct {
minSpace uint64
size uint64
minSpace uint64
size uint64
mountOptions string
}

// Init returns a new BTRFS driver.
Expand Down Expand Up @@ -110,6 +111,8 @@ func parseOptions(opt []string) (btrfsOptions, bool, error) {
}
userDiskQuota = true
options.minSpace = uint64(minSpace)
case "btrfs.mountopt":
options.mountOptions = val
default:
return options, userDiskQuota, fmt.Errorf("Unknown option %s", key)
}
Expand Down
4 changes: 2 additions & 2 deletions drivers/devmapper/deviceset.go
Original file line number Diff line number Diff line change
Expand Up @@ -2664,6 +2664,8 @@ func NewDeviceSet(root string, doInit bool, options []string, uidMaps, gidMaps [
}
key = strings.ToLower(key)
switch key {
case "devicemapper.mountopt":
devices.mountOptions = joinMountOptions(devices.mountOptions, val)
case "dm.basesize":
size, err := units.RAMInBytes(val)
if err != nil {
Expand All @@ -2690,8 +2692,6 @@ func NewDeviceSet(root string, doInit bool, options []string, uidMaps, gidMaps [
devices.filesystem = val
case "dm.mkfsarg":
devices.mkfsArgs = append(devices.mkfsArgs, val)
case "dm.mountopt":
devices.mountOptions = joinMountOptions(devices.mountOptions, val)
case "dm.metadatadev":
devices.metadataDevice = val
case "dm.datadev":
Expand Down
6 changes: 6 additions & 0 deletions drivers/overlay/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type overlayOptions struct {
overrideKernelCheck bool
imageStores []string
quota quota.Quota
mountOptions string
}

// Driver contains information about the home directory and the list of active mounts that are created using this driver.
Expand Down Expand Up @@ -203,6 +204,8 @@ func parseOptions(options []string) (*overlayOptions, error) {
if err != nil {
return nil, err
}
case ".mountopt", "overlay.mountopt", "overlay2.mountopt":
o.mountOptions = val
case ".size", "overlay.size", "overlay2.size":
logrus.Debugf("overlay: size=%s", val)
size, err := units.RAMInBytes(val)
Expand Down Expand Up @@ -666,6 +669,9 @@ func (d *Driver) Get(id, mountLabel string) (_ string, retErr error) {
if len(mountData) > pageSize {
//FIXME: We need to figure out to get this to work with additional stores
opts = fmt.Sprintf("lowerdir=%s,upperdir=%s,workdir=%s", strings.Join(relLowers, ":"), path.Join(id, "diff"), path.Join(id, "work"))
if d.options.mountOptions != "" {
opts = fmt.Sprintf("%s,%s", d.options.mountOptions, opts)
}
mountData = label.FormatMountLabel(opts, mountLabel)
if len(mountData) > pageSize {
return "", fmt.Errorf("cannot mount layer, mount label too large %d", len(mountData))
Expand Down
3 changes: 3 additions & 0 deletions drivers/vfs/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
d.homes = append(d.homes, strings.Split(option[12:], ",")...)
continue
}
if strings.HasPrefix(option, "vfs.mountopt=") {
return nil, fmt.Errorf("vfs driver does not support mount options")
}
}
return graphdriver.NewNaiveDiffDriver(d, graphdriver.NewNaiveLayerIDMapUpdater(d)), nil
}
Expand Down
8 changes: 8 additions & 0 deletions drivers/windows/windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ type Driver struct {
func InitFilter(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) {
logrus.Debugf("WindowsGraphDriver InitFilter at %s", home)

for _, option := range options {
if strings.HasPrefix(option, "windows.mountopt=") {
return nil, fmt.Errorf("windows driver does not support mount options")
} else {
return nil, fmt.Errorf("option %s not supported", option)
}
}

fsType, err := getFileSystemType(string(home[0]))
if err != nil {
return nil, err
Expand Down
10 changes: 8 additions & 2 deletions drivers/zfs/zfs.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,9 @@ import (
)

type zfsOptions struct {
fsName string
mountPath string
fsName string
mountPath string
mountOptions string
}

func init() {
Expand Down Expand Up @@ -134,6 +135,8 @@ func parseOptions(opt []string) (zfsOptions, error) {
switch key {
case "zfs.fsname":
options.fsName = val
case "zfs.mountopt":
options.mountOptions = val
default:
return options, fmt.Errorf("Unknown option %s", key)
}
Expand Down Expand Up @@ -300,6 +303,9 @@ func (d *Driver) create(id, parent string, storageOpt map[string]string) error {
}
if parent == "" {
mountoptions := map[string]string{"mountpoint": "legacy"}
if d.options.mountOptions != "" {
mountoptions[d.options.mountOptions] = ""
}
fs, err := zfs.CreateFilesystem(name, mountoptions)
if err == nil {
err = setQuota(name, quota)
Expand Down
6 changes: 3 additions & 3 deletions storage.conf
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ size = ""
# OverrideKernelCheck tells the driver to ignore kernel checks based on kernel version
override_kernel_check = "false"

# mountopt specifies extra mount options used when mounting
mountopt = "nodev"

# Remap-UIDs/GIDs is the mapping from UIDs/GIDs as they should appear inside of
# a container, to UIDs/GIDs as they should appear outside of the container, and
# the length of the range of UIDs/GIDs. Additional mapped sets can be listed
Expand Down Expand Up @@ -99,9 +102,6 @@ override_kernel_check = "false"
# device.
# mkfsarg = ""

# mountopt specifies extra mount options used when mounting the thin devices.
# mountopt = ""

# use_deferred_removal marks devicemapper block device for deferred removal.
# If the thinpool is in use when the driver attempts to remove it, the driver
# tells the kernel to remove it as soon as possible. Note this does not free
Expand Down
12 changes: 5 additions & 7 deletions store.go
Original file line number Diff line number Diff line change
Expand Up @@ -2638,10 +2638,6 @@ type ThinpoolOptionsConfig struct {
// basedevice.
MkfsArg string `toml:"mkfsarg"`

// MountOpt specifies extra mount options used when mounting the thin
// devices.
MountOpt string `toml:"mountopt"`

// UseDeferredDeletion marks device for deferred deletion
UseDeferredDeletion string `toml:"use_deferred_deletion"`

Expand Down Expand Up @@ -2680,6 +2676,8 @@ type OptionsConfig struct {
RemapGroup string `toml:"remap-group"`
// Thinpool container options to be handed to thinpool drivers
Thinpool struct{ ThinpoolOptionsConfig } `toml:"thinpool"`
// MountOpt specifies extra mount options used when mounting
MountOpt string `toml:"mountopt"`
}

// TOML-friendly explicit tables used for conversions.
Expand Down Expand Up @@ -2752,9 +2750,6 @@ func init() {
if config.Storage.Options.Thinpool.MkfsArg != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("dm.mkfsarg=%s", config.Storage.Options.Thinpool.MkfsArg))
}
if config.Storage.Options.Thinpool.MountOpt != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("dm.mountopt=%s", config.Storage.Options.Thinpool.MountOpt))
}
if config.Storage.Options.Thinpool.UseDeferredDeletion != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("dm.use_deferred_deletion=%s", config.Storage.Options.Thinpool.UseDeferredDeletion))
}
Expand All @@ -2770,6 +2765,9 @@ func init() {
if config.Storage.Options.Size != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("%s.size=%s", config.Storage.Driver, config.Storage.Options.Size))
}
if config.Storage.Options.MountOpt != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("%s.mountopt=%s", config.Storage.Driver, config.Storage.Options.MountOpt))
}
if config.Storage.Options.OverrideKernelCheck != "" {
DefaultStoreOptions.GraphDriverOptions = append(DefaultStoreOptions.GraphDriverOptions, fmt.Sprintf("%s.override_kernel_check=%s", config.Storage.Driver, config.Storage.Options.OverrideKernelCheck))
}
Expand Down

0 comments on commit 8f977dd

Please sign in to comment.