Set base mount options for bind mounts from base system

If I mount, say, /usr/bin into my container - I expect to be able
to run the executables in that mount. Unconditionally applying
noexec would be a bad idea.

Before my patches to change mount options and allow exec/dev/suid
being set explicitly, we inferred the mount options from where on
the base system the mount originated, and the options it had
there. Implement the same functionality for the new option
handling.

There's a lot of performance left on the table here, but I don't
know that this is ever going to take enough time to make it worth
optimizing.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

4 files changed, 67 insertions, 28 deletions

diff --git a/libpod/options.go b/libpod/options.go
index f4bf536b3..6df1ca5be 100644
--- a/libpod/options.go
+++ b/libpod/options.go
@@ -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)
 			}
diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go
index 779fb5290..44bbda885 100644
--- a/pkg/spec/spec.go
+++ b/pkg/spec/spec.go
@@ -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"
@@ -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{
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go
index 9f4248924..7ed21a055 100644
--- a/pkg/spec/storage.go
+++ b/pkg/spec/storage.go
@@ -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"
@@ -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
 	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
 			}
@@ -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
+}
diff --git a/pkg/util/mountOpts.go b/pkg/util/mountOpts.go
index 81dc4cefd..8accd4697 100644
--- a/pkg/util/mountOpts.go
+++ b/pkg/util/mountOpts.go
@@ -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
 	)
@@ -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 {