Skip to content

Commit

Permalink
Add support for 'exec', 'suid', 'dev' mount flags
Browse files Browse the repository at this point in the history 
Previously, we explicitly set noexec/nosuid/nodev on every mount,
with no ability to disable them. The 'mount' command on Linux
will accept their inverses without complaint, though - 'noexec'
is counteracted by 'exec', 'nosuid' by 'suid', etc. Add support
for passing these options at the command line to disable our
explicit forcing of security options.

This also cleans up mount option handling significantly. We are
still parsing options in more than one place, which isn't good,
but option parsing for bind and tmpfs mounts has been unified.

Fixes: containers#3819
Fixes: containers#3803

Signed-off-by: Matthew Heon <matthew.heon@pm.me>
  • Loading branch information
@mheon
mheon committed Aug 22, 2019
1 parent 028a570 commit 8f0162f
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 126 deletions.
7 changes: 6 additions & 1 deletion libpod/options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1360,10 +1360,15 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
}
destinations[vol.Dest] = true

mountOpts, err := util.ProcessOptions(vol.Options, false)
if err != nil {
return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
}

ctr.config.NamedVolumes = append(ctr.config.NamedVolumes, &ContainerNamedVolume{
Name: vol.Name,
Dest: vol.Dest,
Options: util.ProcessOptions(vol.Options),
Options: mountOpts,
})
}

Expand Down
43 changes: 5 additions & 38 deletions pkg/spec/spec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,11 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
// BIND MOUNTS
configSpec.Mounts = supercedeUserMounts(userMounts, configSpec.Mounts)
// Process mounts to ensure correct options
configSpec.Mounts = initFSMounts(configSpec.Mounts)
finalMounts, err := initFSMounts(configSpec.Mounts)
if err != nil {
return nil, err
}
configSpec.Mounts = finalMounts

// BLOCK IO
blkio, err := config.CreateBlockIO()
Expand All @@ -394,43 +398,6 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
}
}

// Make sure that the bind mounts keep options like nosuid, noexec, nodev.
mounts, err := pmount.GetMounts()
if err != nil {
return nil, err
}
for i := range configSpec.Mounts {
m := &configSpec.Mounts[i]
isBind := false
for _, o := range m.Options {
if o == "bind" || o == "rbind" {
isBind = true
break
}
}
if !isBind {
continue
}
mount, err := findMount(m.Source, mounts)
if err != nil {
return nil, err
}
if mount == nil {
continue
}
next_option:
for _, o := range strings.Split(mount.Opts, ",") {
if o == "nosuid" || o == "noexec" || o == "nodev" {
for _, e := range m.Options {
if e == o {
continue next_option
}
}
m.Options = append(m.Options, o)
}
}
}

// Add annotations
if configSpec.Annotations == nil {
configSpec.Annotations = make(map[string]string)
Expand Down
135 changes: 97 additions & 38 deletions pkg/spec/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,30 +160,23 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount,
}

// If requested, add tmpfs filesystems for read-only containers.
// Need to keep track of which we created, so we don't modify options
// for them later...
readonlyTmpfs := map[string]bool{
"/tmp": false,
"/var/tmp": false,
"/run": false,
}
if config.ReadOnlyRootfs && config.ReadOnlyTmpfs {
options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup"}
for dest := range readonlyTmpfs {
readonlyTmpfs := []string{"/tmp", "/var/tmp", "/run"}
options := []string{"rw", "rprivate", "exec", "nosuid", "nodev", "tmpcopyup"}
for _, dest := range readonlyTmpfs {
if _, ok := baseMounts[dest]; ok {
continue
}
localOpts := options
if dest == "/run" {
localOpts = append(localOpts, "noexec", "size=65536k")
localOpts = append(localOpts, "dev", "suid", "noexec", "size=65536k")
}
baseMounts[dest] = spec.Mount{
Destination: dest,
Type: "tmpfs",
Source: "tmpfs",
Options: localOpts,
}
readonlyTmpfs[dest] = true
}
}

Expand All @@ -205,8 +198,8 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount,
// All user-added tmpfs mounts need their options processed.
// Exception: mounts added by the ReadOnlyTmpfs option, which
// contain several exceptions to normal options rules.
if mount.Type == TypeTmpfs && !readonlyTmpfs[mount.Destination] {
opts, err := util.ProcessTmpfsOptions(mount.Options)
if mount.Type == TypeTmpfs {
opts, err := util.ProcessOptions(mount.Options, true)
if err != nil {
return nil, nil, err
}
Expand All @@ -226,9 +219,6 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount,
finalVolumes = append(finalVolumes, volume)
}

logrus.Debugf("Got mounts: %v", finalMounts)
logrus.Debugf("Got volumes: %v", finalVolumes)

return finalMounts, finalVolumes, nil
}

Expand All @@ -250,14 +240,17 @@ func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string]
splitVol = strings.SplitN(vol, ":", 2)
)
if len(splitVol) == 2 {
if strings.Contains(splitVol[1], "Z") ||
strings.Contains(splitVol[1], "private") ||
strings.Contains(splitVol[1], "slave") ||
strings.Contains(splitVol[1], "shared") {
return nil, nil, errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", splitVol[1])
splitOpts := strings.Split(splitVol[1], ",")
for _, checkOpt := range splitOpts {
switch checkOpt {
case "z", "ro", "rw":
// Do nothing, these are valid options
default:
return nil, nil, errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z'", splitVol[1])
}
}

if options, err = parse.ValidateVolumeOpts(strings.Split(splitVol[1], ",")); err != nil {
if options, err = parse.ValidateVolumeOpts(splitOpts); err != nil {
return nil, nil, err
}
}
Expand Down Expand Up @@ -403,9 +396,7 @@ func getBindMount(args []string) (spec.Mount, error) {
Type: TypeBind,
}

setSource := false
setDest := false
setRORW := false
var setSource, setDest, setRORW, setSuid, setDev, setExec bool

for _, val := range args {
kv := strings.Split(val, "=")
Expand Down Expand Up @@ -440,9 +431,23 @@ func getBindMount(args []string) (spec.Mount, error) {
} else {
return newMount, errors.Wrapf(optionArgError, "badly formatted option %q", val)
}
case "nosuid", "nodev", "noexec":
// TODO: detect duplication of these options.
// (Is this necessary?)
case "nosuid", "suid":
if setSuid {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'nosuid' and 'suid' options more than once")
}
setSuid = true
newMount.Options = append(newMount.Options, kv[0])
case "nodev", "dev":
if setDev {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'nodev' and 'dev' options more than once")
}
setDev = true
newMount.Options = append(newMount.Options, kv[0])
case "noexec", "exec":
if setExec {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'noexec' and 'exec' options more than once")
}
setExec = true
newMount.Options = append(newMount.Options, kv[0])
case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z":
newMount.Options = append(newMount.Options, kv[0])
Expand Down Expand Up @@ -497,12 +502,34 @@ func getTmpfsMount(args []string) (spec.Mount, error) {
Source: TypeTmpfs,
}

setDest := false
var setDest, setRORW, setSuid, setDev, setExec bool

for _, val := range args {
kv := strings.Split(val, "=")
switch kv[0] {
case "ro", "nosuid", "nodev", "noexec":
case "ro", "rw":
if setRORW {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'ro' and 'rw' options more than once")
}
setRORW = true
newMount.Options = append(newMount.Options, kv[0])
case "nosuid", "suid":
if setSuid {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'nosuid' and 'suid' options more than once")
}
setSuid = true
newMount.Options = append(newMount.Options, kv[0])
case "nodev", "dev":
if setDev {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'nodev' and 'dev' options more than once")
}
setDev = true
newMount.Options = append(newMount.Options, kv[0])
case "noexec", "exec":
if setExec {
return newMount, errors.Wrapf(optionArgError, "cannot pass 'noexec' and 'exec' options more than once")
}
setExec = true
newMount.Options = append(newMount.Options, kv[0])
case "tmpfs-mode":
if len(kv) == 1 {
Expand Down Expand Up @@ -543,14 +570,34 @@ func getTmpfsMount(args []string) (spec.Mount, error) {
func getNamedVolume(args []string) (*libpod.ContainerNamedVolume, error) {
newVolume := new(libpod.ContainerNamedVolume)

setSource := false
setDest := false
var setSource, setDest, setRORW, setSuid, setDev, setExec bool

for _, val := range args {
kv := strings.Split(val, "=")
switch kv[0] {
case "ro", "nosuid", "nodev", "noexec":
// TODO: detect duplication of these options
case "ro", "rw":
if setRORW {
return nil, errors.Wrapf(optionArgError, "cannot pass 'ro' and 'rw' options more than once")
}
setRORW = true
newVolume.Options = append(newVolume.Options, kv[0])
case "nosuid", "suid":
if setSuid {
return nil, errors.Wrapf(optionArgError, "cannot pass 'nosuid' and 'suid' options more than once")
}
setSuid = true
newVolume.Options = append(newVolume.Options, kv[0])
case "nodev", "dev":
if setDev {
return nil, errors.Wrapf(optionArgError, "cannot pass 'nodev' and 'dev' options more than once")
}
setDev = true
newVolume.Options = append(newVolume.Options, kv[0])
case "noexec", "exec":
if setExec {
return nil, errors.Wrapf(optionArgError, "cannot pass 'noexec' and 'exec' options more than once")
}
setExec = true
newVolume.Options = append(newVolume.Options, kv[0])
case "volume-label":
return nil, errors.Errorf("the --volume-label option is not presently implemented")
Expand Down Expand Up @@ -692,6 +739,9 @@ func (config *CreateConfig) getTmpfsMounts() (map[string]spec.Mount, error) {
var options []string
spliti := strings.Split(i, ":")
destPath := spliti[0]
if err := parse.ValidateVolumeCtrDir(spliti[0]); err != nil {
return nil, err
}
if len(spliti) > 1 {
options = strings.Split(spliti[1], ",")
}
Expand Down Expand Up @@ -775,16 +825,25 @@ func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M
}

// Ensure mount options on all mounts are correct
func initFSMounts(inputMounts []spec.Mount) []spec.Mount {
func initFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
var mounts []spec.Mount
for _, m := range inputMounts {
if m.Type == TypeBind {
m.Options = util.ProcessOptions(m.Options)
opts, err := util.ProcessOptions(m.Options, false)
if err != nil {
return nil, err
}
m.Options = opts
}
if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" {
m.Options = append(m.Options, "tmpcopyup")
opts, err := util.ProcessOptions(m.Options, true)
if err != nil {
return nil, err
}
m.Options = opts
}

mounts = append(mounts, m)
}
return mounts
return mounts, nil
}
Loading

0 comments on commit 8f0162f

Please sign in to comment.