diff --git a/libpod/options.go b/libpod/options.go index a7ddbec34379..f4bf536b3b19 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -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, }) } diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index 156d6849df22..779fb5290f2d 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -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() @@ -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) diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go index b634f4cac189..bc06945f09f3 100644 --- a/pkg/spec/storage.go +++ b/pkg/spec/storage.go @@ -160,22 +160,16 @@ 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, @@ -183,7 +177,6 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, Source: "tmpfs", Options: localOpts, } - readonlyTmpfs[dest] = true } } @@ -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 } @@ -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 } @@ -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 } } @@ -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, "=") @@ -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]) @@ -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 { @@ -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") @@ -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], ",") } @@ -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 } diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go index 9b2c734c0079..81dc4cefdab4 100644 --- a/pkg/util/mountOpts.go +++ b/pkg/util/mountOpts.go @@ -13,88 +13,101 @@ var ( ErrDupeMntOption = errors.Errorf("duplicate option passed") ) -// ProcessOptions parses the options for a bind mount and ensures that they are -// sensible and follow convention. -func ProcessOptions(options []string) []string { +// ProcessOptions parses the options for a bind or tmpfs mount and ensures that +// they are sensible and follow convention. The isTmpfs variable controls +// whether extra, tmpfs-specific options will be allowed. +func ProcessOptions(options []string, isTmpfs bool) ([]string, error) { var ( - foundbind, foundrw, foundro bool - rootProp string + foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind bool ) - for _, opt := range options { - switch opt { - case "bind", "rbind": - foundbind = true - case "ro": - foundro = true - case "rw": - foundrw = true - case "private", "rprivate", "slave", "rslave", "shared", "rshared": - rootProp = opt - } - } - if !foundbind { - options = append(options, "rbind") - } - if !foundrw && !foundro { - options = append(options, "rw") - } - if rootProp == "" { - options = append(options, "rprivate") - } - return options -} - -// ProcessTmpfsOptions parses the options for a tmpfs mountpoint and ensures -// that they are sensible and follow convention. -func ProcessTmpfsOptions(options []string) ([]string, error) { - var ( - foundWrite, foundSize, foundProp, foundMode bool - ) - - baseOpts := []string{"noexec", "nosuid", "nodev"} for _, opt := range options { // Some options have parameters - size, mode splitOpt := strings.SplitN(opt, "=", 2) switch splitOpt[0] { + case "exec", "noexec": + if foundExec { + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'noexec' and 'exec' can be used") + } + foundExec = true + case "suid", "nosuid": + if foundSuid { + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'nosuid' and 'suid' can be used") + } + foundSuid = true + case "nodev", "dev": + if foundDev { + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'nodev' and 'dev' can be used") + } + foundDev = true case "rw", "ro": if foundWrite { - return nil, errors.Wrapf(ErrDupeMntOption, "only one of rw and ro can be used") + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'rw' and 'ro' can be used") } foundWrite = true - baseOpts = append(baseOpts, opt) case "private", "rprivate", "slave", "rslave", "shared", "rshared": if foundProp { return nil, errors.Wrapf(ErrDupeMntOption, "only one root propagation mode can be used") } foundProp = true - baseOpts = append(baseOpts, opt) case "size": + if !isTmpfs { + return nil, errors.Wrapf(ErrBadMntOption, "the 'size' option is only allowed with tmpfs mounts") + } if foundSize { return nil, errors.Wrapf(ErrDupeMntOption, "only one tmpfs size can be specified") } foundSize = true - baseOpts = append(baseOpts, opt) case "mode": + if !isTmpfs { + return nil, errors.Wrapf(ErrBadMntOption, "the 'mode' option is only allowed with tmpfs mounts") + } if foundMode { return nil, errors.Wrapf(ErrDupeMntOption, "only one tmpfs mode can be specified") } foundMode = true - baseOpts = append(baseOpts, opt) - case "noexec", "nodev", "nosuid": - // Do nothing. We always include these even if they are - // not explicitly requested. + case "tmpcopyup": + if !isTmpfs { + return nil, errors.Wrapf(ErrBadMntOption, "the 'tmpcopyup' option is only allowed with tmpfs mounts") + } + if foundCopyUp { + return nil, errors.Wrapf(ErrDupeMntOption, "the 'tmpcopyup' option can only be set once") + } + foundCopyUp = true + case "bind", "rbind": + if isTmpfs { + return nil, errors.Wrapf(ErrBadMntOption, "the 'bind' and 'rbind' options are not allowed with tmpfs mounts") + } + if foundBind { + return nil, errors.Wrapf(ErrDupeMntOption, "only one of 'rbind' and 'bind' can be used") + } + foundBind = true default: - return nil, errors.Wrapf(ErrBadMntOption, "unknown tmpfs option %q", opt) + return nil, errors.Wrapf(ErrBadMntOption, "unknown mount option %q", opt) } } if !foundWrite { - baseOpts = append(baseOpts, "rw") + options = append(options, "rw") } if !foundProp { - baseOpts = append(baseOpts, "rprivate") + options = append(options, "rprivate") + } + if !foundExec { + options = append(options, "noexec") + } + if !foundSuid { + options = append(options, "nosuid") + } + if !foundDev { + options = append(options, "nodev") + } + if isTmpfs && !foundCopyUp { + options = append(options, "tmpcopyup") + } + if !isTmpfs && !foundBind { + options = append(options, "rbind") } - return baseOpts, nil + return options, nil }