Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow suid, exec, dev mount options to cancel nosuid/noexec/nodev #3876

Merged
merged 8 commits into from
Sep 4, 2019
2 changes: 1 addition & 1 deletion libpod/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption {
}
destinations[vol.Dest] = true

mountOpts, err := util.ProcessOptions(vol.Options, false)
mountOpts, err := util.ProcessOptions(vol.Options, false, nil)
if err != nil {
return errors.Wrapf(err, "error processing options for named volume %q mounted at %q", vol.Name, vol.Dest)
}
Expand Down
21 changes: 0 additions & 21 deletions pkg/spec/spec.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,11 @@ package createconfig

import (
"os"
"path/filepath"
"strings"

"github.com/containers/libpod/libpod"
"github.com/containers/libpod/pkg/cgroups"
"github.com/containers/libpod/pkg/rootless"
pmount "github.com/containers/storage/pkg/mount"
"github.com/docker/docker/oci/caps"
"github.com/docker/go-units"
"github.com/opencontainers/runc/libcontainer/user"
Expand Down Expand Up @@ -457,25 +455,6 @@ func (config *CreateConfig) createConfigToOCISpec(runtime *libpod.Runtime, userM
return configSpec, nil
}

func findMount(target string, mounts []*pmount.Info) (*pmount.Info, error) {
var err error
target, err = filepath.Abs(target)
if err != nil {
return nil, errors.Wrapf(err, "cannot resolve %s", target)
}
var bestSoFar *pmount.Info
for _, i := range mounts {
if bestSoFar != nil && len(bestSoFar.Mountpoint) > len(i.Mountpoint) {
// Won't be better than what we have already found
continue
}
if strings.HasPrefix(target, i.Mountpoint) {
bestSoFar = i
}
}
return bestSoFar, nil
}

func blockAccessToKernelFilesystems(config *CreateConfig, g *generate.Generator) {
if !config.Privileged {
for _, mp := range []string{
Expand Down
55 changes: 53 additions & 2 deletions pkg/spec/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/containers/buildah/pkg/parse"
"github.com/containers/libpod/libpod"
"github.com/containers/libpod/pkg/util"
pmount "github.com/containers/storage/pkg/mount"
"github.com/containers/storage/pkg/stringid"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/pkg/errors"
Expand Down Expand Up @@ -816,17 +817,46 @@ 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, error) {
// We need to look up mounts so we can figure out the proper mount flags
// to apply.
systemMounts, err := pmount.GetMounts()
if err != nil {
return nil, errors.Wrapf(err, "error retrieving system mounts to look up mount options")
}

// TODO: We probably don't need to re-build the mounts array
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I think so too. Operating on inputMounts[i] should work.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll leave it off for now, this is large enough already

var mounts []spec.Mount
for _, m := range inputMounts {
if m.Type == TypeBind {
opts, err := util.ProcessOptions(m.Options, false)
baseMnt, err := findMount(m.Destination, systemMounts)
if err != nil {
return nil, errors.Wrapf(err, "error looking up mountpoint for mount %s", m.Destination)
}
var noexec, nosuid, nodev bool
for _, baseOpt := range strings.Split(baseMnt.Opts, ",") {
switch baseOpt {
case "noexec":
noexec = true
case "nosuid":
nosuid = true
case "nodev":
nodev = true
}
}

defaultMountOpts := new(util.DefaultMountOptions)
defaultMountOpts.Noexec = noexec
defaultMountOpts.Nosuid = nosuid
defaultMountOpts.Nodev = nodev

opts, err := util.ProcessOptions(m.Options, false, defaultMountOpts)
if err != nil {
return nil, err
}
m.Options = opts
}
if m.Type == TypeTmpfs && filepath.Clean(m.Destination) != "/dev" {
opts, err := util.ProcessOptions(m.Options, true)
opts, err := util.ProcessOptions(m.Options, true, nil)
if err != nil {
return nil, err
}
Expand All @@ -837,3 +867,24 @@ func initFSMounts(inputMounts []spec.Mount) ([]spec.Mount, error) {
}
return mounts, nil
}

// TODO: We could make this a bit faster by building a tree of the mountpoints
// and traversing it to identify the correct mount.
func findMount(target string, mounts []*pmount.Info) (*pmount.Info, error) {
var err error
target, err = filepath.Abs(target)
if err != nil {
return nil, errors.Wrapf(err, "cannot resolve %s", target)
}
var bestSoFar *pmount.Info
for _, i := range mounts {
if bestSoFar != nil && len(bestSoFar.Mountpoint) > len(i.Mountpoint) {
// Won't be better than what we have already found
continue
}
if strings.HasPrefix(target, i.Mountpoint) {
bestSoFar = i
}
}
return bestSoFar, nil
}
17 changes: 13 additions & 4 deletions pkg/util/mountOpts.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,19 @@ var (
ErrDupeMntOption = errors.Errorf("duplicate option passed")
)

// DefaultMountOptions sets default mount options for ProcessOptions.
type DefaultMountOptions struct {
Noexec bool
Nosuid bool
Nodev bool
}

// 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) {
// The defaults variable controls default mount options that will be set. If it
// is not included, they will be set unconditionally.
func ProcessOptions(options []string, isTmpfs bool, defaults *DefaultMountOptions) ([]string, error) {
var (
foundWrite, foundSize, foundProp, foundMode, foundExec, foundSuid, foundDev, foundCopyUp, foundBind bool
)
Expand Down Expand Up @@ -93,13 +102,13 @@ func ProcessOptions(options []string, isTmpfs bool) ([]string, error) {
if !foundProp {
options = append(options, "rprivate")
}
if !foundExec {
if !foundExec && (defaults == nil || defaults.Noexec) {
options = append(options, "noexec")
}
if !foundSuid {
if !foundSuid && (defaults == nil || defaults.Nosuid) {
options = append(options, "nosuid")
}
if !foundDev {
if !foundDev && (defaults == nil || defaults.Nodev) {
options = append(options, "nodev")
}
if isTmpfs && !foundCopyUp {
Expand Down