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]>
rhatdan committed Jul 18, 2018
1 parent cb8d712 commit 8b1a0f8
Showing 10 changed files with 67 additions and 18 deletions.
8 changes: 4 additions & 4 deletions docs/containers-storage.conf.5.md
Original file line number Diff line number Diff line change
@@ -58,6 +58,10 @@ The `storage.options` table supports the following options:
**mount_program**=""
Specifies the path to a custom program to use instead for mounting the file system.

**mountopt**=""

Comma separated list of default options to be used to mount container images. Suggested value "nodev".

[storage.options.thinpool]

Storage Options for thinpool
@@ -112,10 +116,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).
33 changes: 27 additions & 6 deletions drivers/aufs/aufs.go
Original file line number Diff line number Diff line change
@@ -42,6 +42,7 @@ import (
"github.com/containers/storage/pkg/idtools"
"github.com/containers/storage/pkg/locker"
mountpk "github.com/containers/storage/pkg/mount"
"github.com/containers/storage/pkg/parsers"
"github.com/containers/storage/pkg/system"
rsystem "github.com/opencontainers/runc/libcontainer/system"
"github.com/opencontainers/selinux/go-selinux/label"
@@ -77,6 +78,7 @@ type Driver struct {
pathCache map[string]string
naiveDiff graphdriver.DiffDriver
locker *locker.Locker
mountOptions string
}

// Init returns a new AUFS driver.
@@ -103,19 +105,34 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
return nil, errors.Wrapf(graphdriver.ErrIncompatibleFS, "AUFS is not supported over %q", backingFs)
}

var mountOptions string
for _, option := range options {
key, val, err := parsers.ParseKeyValueOpt(option)
if err != nil {
return nil, err
}
key = strings.ToLower(key)
switch key {
case "aufs.mountopt":
mountOptions = val
default:
return nil, fmt.Errorf("option %s not supported", option)
}
}
paths := []string{
"mnt",
"diff",
"layers",
}

a := &Driver{
root: root,
uidMaps: uidMaps,
gidMaps: gidMaps,
pathCache: make(map[string]string),
ctr: graphdriver.NewRefCounter(graphdriver.NewFsChecker(graphdriver.FsMagicAufs)),
locker: locker.New(),
root: root,
uidMaps: uidMaps,
gidMaps: gidMaps,
pathCache: make(map[string]string),
ctr: graphdriver.NewRefCounter(graphdriver.NewFsChecker(graphdriver.FsMagicAufs)),
locker: locker.New(),
mountOptions: mountOptions,
}

rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
@@ -653,6 +670,10 @@ func (a *Driver) aufsMount(ro []string, rw, target, mountLabel string) (err erro
}

opts := "dio,xino=/dev/shm/aufs.xino"
if a.mountOptions != "" {
opts += fmt.Sprintf(",%s", a.mountOptions)
}

if useDirperm() {
opts += ",dirperm1"
}
2 changes: 2 additions & 0 deletions drivers/btrfs/btrfs.go
Original file line number Diff line number Diff line change
@@ -110,6 +110,8 @@ func parseOptions(opt []string) (btrfsOptions, bool, error) {
}
userDiskQuota = true
options.minSpace = uint64(minSpace)
case "btrfs.mountopt":
return options, userDiskQuota, fmt.Errorf("btrfs driver does not support mount options")
default:
return options, userDiskQuota, fmt.Errorf("Unknown option %s", key)
}
2 changes: 1 addition & 1 deletion drivers/devmapper/deviceset.go
Original file line number Diff line number Diff line change
@@ -2690,7 +2690,7 @@ 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":
case "dm.mountopt", "devicemapper.mountopt":
devices.mountOptions = joinMountOptions(devices.mountOptions, val)
case "dm.metadatadev":
devices.metadataDevice = val
6 changes: 6 additions & 0 deletions drivers/overlay/overlay.go
Original file line number Diff line number Diff line change
@@ -88,6 +88,7 @@ type overlayOptions struct {
mountProgram string
ostreeRepo string
skipMountHome bool
mountOptions string
}

// Driver contains information about the home directory and the list of active mounts that are created using this driver.
@@ -222,6 +223,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)
@@ -716,6 +719,9 @@ func (d *Driver) Get(id, mountLabel string) (_ string, retErr error) {
if len(mountData) > pageSize || d.options.mountProgram != "" {
//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))
3 changes: 3 additions & 0 deletions drivers/vfs/driver.go
Original file line number Diff line number Diff line change
@@ -55,6 +55,9 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap
}
d.ostreeRepo = option[13:]
}
if strings.HasPrefix(option, "vfs.mountopt=") {
return nil, fmt.Errorf("vfs driver does not support mount options")
}
}
if d.ostreeRepo != "" {
rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)
8 changes: 8 additions & 0 deletions drivers/windows/windows.go
Original file line number Diff line number Diff line change
@@ -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
9 changes: 6 additions & 3 deletions drivers/zfs/zfs.go
Original file line number Diff line number Diff line change
@@ -24,8 +24,9 @@ import (
)

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

func init() {
@@ -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)
}
@@ -364,7 +367,7 @@ func (d *Driver) Get(id, mountLabel string) (string, error) {
}

filesystem := d.zfsPath(id)
options := label.FormatMountLabel("", mountLabel)
options := label.FormatMountLabel(d.options.mountOptions, mountLabel)
logrus.Debugf(`[zfs] mount("%s", "%s", "%s")`, filesystem, mountpoint, options)

rootUID, rootGID, err := idtools.GetRootUIDGID(d.uidMaps, d.gidMaps)
6 changes: 3 additions & 3 deletions storage.conf
Original file line number Diff line number Diff line change
@@ -32,6 +32,9 @@ size = ""
# OverrideKernelCheck tells the driver to ignore kernel checks based on kernel version
override_kernel_check = "false"

# mountopt specifies comma separated list of extra mount options
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
@@ -103,9 +106,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
8 changes: 7 additions & 1 deletion store.go
Original file line number Diff line number Diff line change
@@ -3015,6 +3015,9 @@ type OptionsConfig struct {

// Alternative program to use for the mount of the file system
MountProgram string `toml:"mount_program"`

// MountOpt specifies extra mount options used when mounting
MountOpt string `toml:"mountopt"`
}

// TOML-friendly explicit tables used for conversions.
@@ -3086,7 +3089,7 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("dm.mkfsarg=%s", config.Storage.Options.Thinpool.MkfsArg))
}
if config.Storage.Options.Thinpool.MountOpt != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("dm.mountopt=%s", config.Storage.Options.Thinpool.MountOpt))
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mountopt=%s", config.Storage.Driver, config.Storage.Options.Thinpool.MountOpt))
}
if config.Storage.Options.Thinpool.UseDeferredDeletion != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("dm.use_deferred_deletion=%s", config.Storage.Options.Thinpool.UseDeferredDeletion))
@@ -3112,6 +3115,9 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) {
if config.Storage.Options.MountProgram != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mount_program=%s", config.Storage.Driver, config.Storage.Options.MountProgram))
}
if config.Storage.Options.MountOpt != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.mountopt=%s", config.Storage.Driver, config.Storage.Options.MountOpt))
}
if config.Storage.Options.OverrideKernelCheck != "" {
storeOptions.GraphDriverOptions = append(storeOptions.GraphDriverOptions, fmt.Sprintf("%s.override_kernel_check=%s", config.Storage.Driver, config.Storage.Options.OverrideKernelCheck))
}

0 comments on commit 8b1a0f8

Please sign in to comment.