Fix sandbox and gofer capabilities

Capabilities.Set() adds capabilities,
but doesn't remove existing ones that might have been loaded. Fixed
the code and added tests.

PiperOrigin-RevId: 213726369
Change-Id: Id7fa6fce53abf26c29b13b9157bb4c6616986fba

6 files changed, 197 insertions, 52 deletions

diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go
index 420e57022..59ae5faae 100644
--- a/runsc/boot/fs.go
+++ b/runsc/boot/fs.go
@@ -428,13 +428,13 @@ func parseAndFilterOptions(opts []string, allowedKeys ...string) ([]string, erro
 		kv := strings.Split(o, "=")
 		switch len(kv) {
 		case 1:
-			if contains(allowedKeys, o) {
+			if specutils.ContainsStr(allowedKeys, o) {
 				out = append(out, o)
 				continue
 			}
 			log.Warningf("ignoring unsupported key %q", kv)
 		case 2:
-			if contains(allowedKeys, kv[0]) {
+			if specutils.ContainsStr(allowedKeys, kv[0]) {
 				out = append(out, o)
 				continue
 			}
@@ -540,15 +540,6 @@ func mountFlags(opts []string) fs.MountSourceFlags {
 	return mf
 }
 
-func contains(strs []string, str string) bool {
-	for _, s := range strs {
-		if s == str {
-			return true
-		}
-	}
-	return false
-}
-
 func mustFindFilesystem(name string) fs.Filesystem {
 	fs, ok := fs.FindFilesystem(name)
 	if !ok {
diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD
index f9c091ba2..7c90ff2c5 100644
--- a/runsc/cmd/BUILD
+++ b/runsc/cmd/BUILD
@@ -55,18 +55,27 @@ go_test(
     name = "cmd_test",
     size = "small",
     srcs = [
+        "capability_test.go",
         "delete_test.go",
         "exec_test.go",
     ],
+    data = [
+        "//runsc",
+    ],
     embed = [":cmd"],
     deps = [
         "//pkg/abi/linux",
+        "//pkg/log",
         "//pkg/sentry/control",
         "//pkg/sentry/kernel/auth",
         "//pkg/urpc",
         "//runsc/boot",
+        "//runsc/container",
+        "//runsc/specutils",
+        "//runsc/test/testutil",
         "@com_github_google_go-cmp//cmp:go_default_library",
         "@com_github_google_go-cmp//cmp/cmpopts:go_default_library",
         "@com_github_opencontainers_runtime-spec//specs-go:go_default_library",
+        "@com_github_syndtr_gocapability//capability:go_default_library",
     ],
 )
diff --git a/runsc/cmd/capability.go b/runsc/cmd/capability.go
index e2410d4ad..affbb7ce3 100644
--- a/runsc/cmd/capability.go
+++ b/runsc/cmd/capability.go
@@ -16,56 +16,67 @@ package cmd
 
 import (
 	"fmt"
-	"os"
 
 	specs "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/syndtr/gocapability/capability"
 	"gvisor.googlesource.com/gvisor/pkg/log"
 )
 
+var allCapTypes = []capability.CapType{
+	capability.BOUNDS,
+	capability.EFFECTIVE,
+	capability.PERMITTED,
+	capability.INHERITABLE,
+	capability.AMBIENT,
+}
+
 // applyCaps applies the capabilities in the spec to the current thread.
 //
 // Note that it must be called with current thread locked.
 func applyCaps(caps *specs.LinuxCapabilities) error {
-	setter, err := capability.NewPid2(os.Getpid())
+	// Load current capabilities to trim the ones not permitted.
+	curCaps, err := capability.NewPid2(0)
 	if err != nil {
 		return err
 	}
-	if err := setter.Load(); err != nil {
+	if err := curCaps.Load(); err != nil {
 		return err
 	}
 
-	bounding, err := trimCaps(caps.Bounding, setter)
+	// Create an empty capability set to populate.
+	newCaps, err := capability.NewPid2(0)
 	if err != nil {
 		return err
 	}
-	setter.Set(capability.BOUNDS, bounding...)
 
-	effective, err := trimCaps(caps.Effective, setter)
-	if err != nil {
-		return err
-	}
-	setter.Set(capability.EFFECTIVE, effective...)
-
-	permitted, err := trimCaps(caps.Permitted, setter)
-	if err != nil {
-		return err
+	for _, c := range allCapTypes {
+		if !newCaps.Empty(c) {
+			panic("unloaded capabilities must be empty")
+		}
+		set, err := trimCaps(getCaps(c, caps), curCaps)
+		if err != nil {
+			return err
+		}
+		newCaps.Set(c, set...)
 	}
-	setter.Set(capability.PERMITTED, permitted...)
 
-	inheritable, err := trimCaps(caps.Inheritable, setter)
-	if err != nil {
-		return err
-	}
-	setter.Set(capability.INHERITABLE, inheritable...)
+	return newCaps.Apply(capability.CAPS | capability.BOUNDS | capability.AMBS)
+}
 
-	ambient, err := trimCaps(caps.Ambient, setter)
-	if err != nil {
-		return err
+func getCaps(which capability.CapType, caps *specs.LinuxCapabilities) []string {
+	switch which {
+	case capability.BOUNDS:
+		return caps.Bounding
+	case capability.EFFECTIVE:
+		return caps.Effective
+	case capability.PERMITTED:
+		return caps.Permitted
+	case capability.INHERITABLE:
+		return caps.Inheritable
+	case capability.AMBIENT:
+		return caps.Ambient
 	}
-	setter.Set(capability.AMBIENT, ambient...)
-
-	return setter.Apply(capability.CAPS | capability.BOUNDS | capability.AMBS)
+	panic(fmt.Sprint("invalid capability type:", which))
 }
 
 func trimCaps(names []string, setter capability.Capabilities) ([]capability.Cap, error) {
diff --git a/runsc/cmd/capability_test.go b/runsc/cmd/capability_test.go
new file mode 100644
index 000000000..be9ef2e7b
--- /dev/null
+++ b/runsc/cmd/capability_test.go
@@ -0,0 +1,121 @@
+// Copyright 2018 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cmd
+
+import (
+	"fmt"
+	"os"
+	"testing"
+
+	specs "github.com/opencontainers/runtime-spec/specs-go"
+	"github.com/syndtr/gocapability/capability"
+	"gvisor.googlesource.com/gvisor/pkg/log"
+	"gvisor.googlesource.com/gvisor/runsc/boot"
+	"gvisor.googlesource.com/gvisor/runsc/container"
+	"gvisor.googlesource.com/gvisor/runsc/specutils"
+	"gvisor.googlesource.com/gvisor/runsc/test/testutil"
+)
+
+func init() {
+	log.SetLevel(log.Debug)
+	if err := testutil.ConfigureExePath(); err != nil {
+		panic(err.Error())
+	}
+}
+
+func checkProcessCaps(pid int, wantCaps *specs.LinuxCapabilities) error {
+	curCaps, err := capability.NewPid2(pid)
+	if err != nil {
+		return fmt.Errorf("capability.NewPid2(%d) failed: %v", pid, err)
+	}
+	if err := curCaps.Load(); err != nil {
+		return fmt.Errorf("unable to load capabilities: %v", err)
+	}
+	fmt.Printf("Capabilities (PID: %d): %v\n", pid, curCaps)
+
+	for _, c := range allCapTypes {
+		if err := checkCaps(c, curCaps, wantCaps); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func checkCaps(which capability.CapType, curCaps capability.Capabilities, wantCaps *specs.LinuxCapabilities) error {
+	wantNames := getCaps(which, wantCaps)
+	for name, c := range capFromName {
+		want := specutils.ContainsStr(wantNames, name)
+		got := curCaps.Get(which, c)
+		if want != got {
+			if want {
+				return fmt.Errorf("capability %v:%s should be set", which, name)
+			}
+			return fmt.Errorf("capability %v:%s should NOT be set", which, name)
+		}
+	}
+	return nil
+}
+
+func TestCapabilities(t *testing.T) {
+	stop := testutil.StartReaper()
+	defer stop()
+
+	spec := testutil.NewSpecWithArgs("/bin/sleep", "10000")
+	caps := []string{
+		"CAP_CHOWN",
+		"CAP_SYS_PTRACE", // ptrace is added due to the platform choice.
+	}
+	spec.Process.Capabilities = &specs.LinuxCapabilities{
+		Permitted:   caps,
+		Bounding:    caps,
+		Effective:   caps,
+		Inheritable: caps,
+	}
+
+	conf := testutil.TestConfig()
+
+	// Use --network=host to make sandbox use spec's capabilities.
+	conf.Network = boot.NetworkHost
+
+	rootDir, bundleDir, err := testutil.SetupContainer(spec, conf)
+	if err != nil {
+		t.Fatalf("error setting up container: %v", err)
+	}
+	defer os.RemoveAll(rootDir)
+	defer os.RemoveAll(bundleDir)
+
+	// Create and start the container.
+	c, err := container.Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "")
+	if err != nil {
+		t.Fatalf("error creating container: %v", err)
+	}
+	defer c.Destroy()
+	if err := c.Start(conf); err != nil {
+		t.Fatalf("error starting container: %v", err)
+	}
+
+	// Check that sandbox and gofer have the proper capabilities.
+	if err := checkProcessCaps(c.Sandbox.Pid, spec.Process.Capabilities); err != nil {
+		t.Error(err)
+	}
+	if err := checkProcessCaps(c.GoferPid, goferCaps); err != nil {
+		t.Error(err)
+	}
+}
+
+func TestMain(m *testing.M) {
+	testutil.RunAsRoot()
+	os.Exit(m.Run())
+}
diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go
index 95926f5f9..fd4eee546 100644
--- a/runsc/cmd/gofer.go
+++ b/runsc/cmd/gofer.go
@@ -31,6 +31,23 @@ import (
 	"gvisor.googlesource.com/gvisor/runsc/specutils"
 )
 
+var caps = []string{
+	"CAP_CHOWN",
+	"CAP_DAC_OVERRIDE",
+	"CAP_DAC_READ_SEARCH",
+	"CAP_FOWNER",
+	"CAP_FSETID",
+	"CAP_SYS_CHROOT",
+}
+
+// goferCaps is the minimal set of capabilities needed by the Gofer to operate
+// on files.
+var goferCaps = &specs.LinuxCapabilities{
+	Bounding:  caps,
+	Effective: caps,
+	Permitted: caps,
+}
+
 // Gofer implements subcommands.Command for the "gofer" command, which starts a
 // filesystem gofer.  This command should not be called directly.
 type Gofer struct {
@@ -72,25 +89,11 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{})
 	}
 
 	if g.applyCaps {
-		// Minimal set of capabilities needed by the Gofer to operate on files.
-		caps := []string{
-			"CAP_CHOWN",
-			"CAP_DAC_OVERRIDE",
-			"CAP_DAC_READ_SEARCH",
-			"CAP_FOWNER",
-			"CAP_FSETID",
-		}
-		lc := &specs.LinuxCapabilities{
-			Bounding:  caps,
-			Effective: caps,
-			Permitted: caps,
-		}
-
 		// Disable caps when calling myself again.
 		// Note: minimal argument handling for the default case to keep it simple.
 		args := os.Args
 		args = append(args, "--apply-caps=false")
-		if err := setCapsAndCallSelf(args, lc); err != nil {
+		if err := setCapsAndCallSelf(args, goferCaps); err != nil {
 			Fatalf("Unable to apply caps: %v", err)
 		}
 		panic("unreachable")
diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go
index fdc9007e0..daf10b875 100644
--- a/runsc/specutils/specutils.go
+++ b/runsc/specutils/specutils.go
@@ -392,3 +392,13 @@ func Mount(src, dst, typ string, flags uint32) error {
 	}
 	return nil
 }
+
+// ContainsStr returns true if 'str' is inside 'strs'.
+func ContainsStr(strs []string, str string) bool {
+	for _, s := range strs {
+		if s == str {
+			return true
+		}
+	}
+	return false
+}