Properly handle default capabilities listed in containers.conf

If user/admin specifies a different list of default capabilties
we need to honor these.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>

4 files changed, 46 insertions, 81 deletions

diff --git a/pkg/specgen/generate/oci.go b/pkg/specgen/generate/oci.go
index c70141109..8136c0993 100644
--- a/pkg/specgen/generate/oci.go
+++ b/pkg/specgen/generate/oci.go
@@ -304,7 +304,7 @@ func SpecGenToOCI(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runt
 	}
 	configSpec := g.Config
 
-	if err := securityConfigureGenerator(s, &g, newImage); err != nil {
+	if err := securityConfigureGenerator(s, &g, newImage, rtc); err != nil {
 		return nil, err
 	}
 
diff --git a/pkg/specgen/generate/security.go b/pkg/specgen/generate/security.go
index e2da9e976..d2229b06f 100644
--- a/pkg/specgen/generate/security.go
+++ b/pkg/specgen/generate/security.go
@@ -4,6 +4,7 @@ import (
 	"strings"
 
 	"github.com/containers/common/pkg/capabilities"
+	"github.com/containers/common/pkg/config"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/image"
 	"github.com/containers/libpod/pkg/specgen"
@@ -55,76 +56,61 @@ func setLabelOpts(s *specgen.SpecGenerator, runtime *libpod.Runtime, pidConfig s
 	return nil
 }
 
-func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image) error {
+func securityConfigureGenerator(s *specgen.SpecGenerator, g *generate.Generator, newImage *image.Image, rtc *config.Config) error {
+	var (
+		caplist []string
+		err     error
+	)
 	// HANDLE CAPABILITIES
 	// NOTE: Must happen before SECCOMP
 	if s.Privileged {
 		g.SetupPrivileged(true)
-	}
-
-	useNotRoot := func(user string) bool {
-		if user == "" || user == "root" || user == "0" {
-			return false
+		caplist = capabilities.AllCapabilities()
+	} else {
+		caplist, err = rtc.Capabilities(s.User, s.CapAdd, s.CapDrop)
+		if err != nil {
+			return err
 		}
-		return true
-	}
-	configSpec := g.Config
-	var err error
-	var caplist []string
-	bounding := configSpec.Process.Capabilities.Bounding
-	if useNotRoot(s.User) {
-		configSpec.Process.Capabilities.Bounding = caplist
-	}
-	caplist, err = capabilities.MergeCapabilities(configSpec.Process.Capabilities.Bounding, s.CapAdd, s.CapDrop)
-	if err != nil {
-		return err
-	}
-	privCapsRequired := []string{}
 
-	// If the container image specifies an label with a
-	// capabilities.ContainerImageLabel then split the comma separated list
-	// of capabilities and record them.  This list indicates the only
-	// capabilities, required to run the container.
-	var capsRequiredRequested []string
-	for key, val := range s.Labels {
-		if util.StringInSlice(key, capabilities.ContainerImageLabels) {
-			capsRequiredRequested = strings.Split(val, ",")
+		privCapsRequired := []string{}
+
+		// If the container image specifies an label with a
+		// capabilities.ContainerImageLabel then split the comma separated list
+		// of capabilities and record them.  This list indicates the only
+		// capabilities, required to run the container.
+		var capsRequiredRequested []string
+		for key, val := range s.Labels {
+			if util.StringInSlice(key, capabilities.ContainerImageLabels) {
+				capsRequiredRequested = strings.Split(val, ",")
+			}
 		}
-	}
-	if !s.Privileged && len(capsRequiredRequested) > 0 {
+		if !s.Privileged && len(capsRequiredRequested) > 0 {
 
-		// Pass capRequiredRequested in CapAdd field to normalize capabilities names
-		capsRequired, err := capabilities.MergeCapabilities(nil, capsRequiredRequested, nil)
-		if err != nil {
-			logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(capsRequired, ","))
-		} else {
-			// Verify all capRequiered are in the capList
-			for _, cap := range capsRequired {
-				if !util.StringInSlice(cap, caplist) {
-					privCapsRequired = append(privCapsRequired, cap)
+			// Pass capRequiredRequested in CapAdd field to normalize capabilities names
+			capsRequired, err := capabilities.MergeCapabilities(nil, capsRequiredRequested, nil)
+			if err != nil {
+				logrus.Errorf("capabilities requested by user or image are not valid: %q", strings.Join(capsRequired, ","))
+			} else {
+				// Verify all capRequiered are in the capList
+				for _, cap := range capsRequired {
+					if !util.StringInSlice(cap, caplist) {
+						privCapsRequired = append(privCapsRequired, cap)
+					}
 				}
 			}
-		}
-		if len(privCapsRequired) == 0 {
-			caplist = capsRequired
-		} else {
-			logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapsRequired, ","))
+			if len(privCapsRequired) == 0 {
+				caplist = capsRequired
+			} else {
+				logrus.Errorf("capabilities requested by user or image are not allowed by default: %q", strings.Join(privCapsRequired, ","))
+			}
 		}
 	}
-
+	configSpec := g.Config
 	configSpec.Process.Capabilities.Bounding = caplist
 	configSpec.Process.Capabilities.Permitted = caplist
 	configSpec.Process.Capabilities.Inheritable = caplist
 	configSpec.Process.Capabilities.Effective = caplist
 	configSpec.Process.Capabilities.Ambient = caplist
-	if useNotRoot(s.User) {
-		caplist, err = capabilities.MergeCapabilities(bounding, s.CapAdd, s.CapDrop)
-		if err != nil {
-			return err
-		}
-	}
-	configSpec.Process.Capabilities.Bounding = caplist
-
 	// HANDLE SECCOMP
 	if s.SeccompProfilePath != "unconfined" {
 		seccompConfig, err := getSeccompConfig(s, configSpec, newImage)
diff --git a/vendor/github.com/containers/buildah/imagebuildah/executor.go b/vendor/github.com/containers/buildah/imagebuildah/executor.go
index a0debc460..02123c822 100644
--- a/vendor/github.com/containers/buildah/imagebuildah/executor.go
+++ b/vendor/github.com/containers/buildah/imagebuildah/executor.go
@@ -113,7 +113,10 @@ func NewExecutor(store storage.Store, options BuildOptions, mainNode *parser.Nod
 	if err != nil {
 		return nil, err
 	}
-	capabilities := defaultContainerConfig.Capabilities("", options.AddCapabilities, options.DropCapabilities)
+	capabilities, err := defaultContainerConfig.Capabilities("", options.AddCapabilities, options.DropCapabilities)
+	if err != nil {
+		return nil, err
+	}
 
 	devices := []configs.Device{}
 	for _, device := range append(defaultContainerConfig.Containers.Devices, options.Devices...) {
diff --git a/vendor/github.com/containers/common/pkg/config/config.go b/vendor/github.com/containers/common/pkg/config/config.go
index bddbee876..0f17c27c9 100644
--- a/vendor/github.com/containers/common/pkg/config/config.go
+++ b/vendor/github.com/containers/common/pkg/config/config.go
@@ -709,7 +709,7 @@ func (c *Config) GetDefaultEnv() []string {
 
 // Capabilities returns the capabilities parses the Add and Drop capability
 // list from the default capabiltiies for the container
-func (c *Config) Capabilities(user string, addCapabilities, dropCapabilities []string) []string {
+func (c *Config) Capabilities(user string, addCapabilities, dropCapabilities []string) ([]string, error) {
 
 	userNotRoot := func(user string) bool {
 		if user == "" || user == "root" || user == "0" {
@@ -718,36 +718,12 @@ func (c *Config) Capabilities(user string, addCapabilities, dropCapabilities []s
 		return true
 	}
 
-	var caps []string
 	defaultCapabilities := c.Containers.DefaultCapabilities
 	if userNotRoot(user) {
 		defaultCapabilities = []string{}
 	}
 
-	mapCap := make(map[string]bool, len(defaultCapabilities))
-	for _, c := range addCapabilities {
-		if strings.ToLower(c) == "all" {
-			defaultCapabilities = capabilities.AllCapabilities()
-			addCapabilities = nil
-			break
-		}
-	}
-
-	for _, c := range append(defaultCapabilities, addCapabilities...) {
-		mapCap[c] = true
-	}
-	for _, c := range dropCapabilities {
-		if "all" == strings.ToLower(c) {
-			return caps
-		}
-		mapCap[c] = false
-	}
-	for cap, add := range mapCap {
-		if add {
-			caps = append(caps, cap)
-		}
-	}
-	return caps
+	return capabilities.MergeCapabilities(defaultCapabilities, addCapabilities, dropCapabilities)
 }
 
 // Device parses device mapping string to a src, dest & permissions string