From 67a2ab1438cdccbe045143bbfaa807cf83110ebc Mon Sep 17 00:00:00 2001 From: Adin Scannell Date: Tue, 3 Sep 2019 22:01:34 -0700 Subject: Impose order on test scripts. The simple test script has gotten out of control. Shard this script into different pieces and attempt to impose order on overall test structure. This change helps lay some of the foundations for future improvements. * The runsc/test directories are moved into just test/. * The runsc/test/testutil package is split into logical pieces. * The scripts/ directory contains new top-level targets. * Each test is now responsible for building targets it requires. * The install functionality is moved into `runsc` itself for simplicity. * The existing kokoro run_tests.sh file now just calls all (can be split). After this change is merged, I will create multiple distinct workflows for Kokoro, one for each of the scripts currently targeted by `run_tests.sh` today, which should dramatically reduce the time-to-run for the Kokoro tests, and provides a better foundation for further improvements to the infrastructure. PiperOrigin-RevId: 267081397 --- runsc/cmd/BUILD | 3 +- runsc/cmd/capability_test.go | 4 +- runsc/cmd/install.go | 210 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 215 insertions(+), 2 deletions(-) create mode 100644 runsc/cmd/install.go (limited to 'runsc/cmd') diff --git a/runsc/cmd/BUILD b/runsc/cmd/BUILD index 5223b9972..250845ad7 100644 --- a/runsc/cmd/BUILD +++ b/runsc/cmd/BUILD @@ -19,6 +19,7 @@ go_library( "exec.go", "gofer.go", "help.go", + "install.go", "kill.go", "list.go", "path.go", @@ -81,7 +82,7 @@ go_test( "//runsc/boot", "//runsc/container", "//runsc/specutils", - "//runsc/test/testutil", + "//runsc/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", diff --git a/runsc/cmd/capability_test.go b/runsc/cmd/capability_test.go index 3ae25a257..0c27f7313 100644 --- a/runsc/cmd/capability_test.go +++ b/runsc/cmd/capability_test.go @@ -15,6 +15,7 @@ package cmd import ( + "flag" "fmt" "os" "testing" @@ -25,7 +26,7 @@ import ( "gvisor.dev/gvisor/runsc/boot" "gvisor.dev/gvisor/runsc/container" "gvisor.dev/gvisor/runsc/specutils" - "gvisor.dev/gvisor/runsc/test/testutil" + "gvisor.dev/gvisor/runsc/testutil" ) func init() { @@ -121,6 +122,7 @@ func TestCapabilities(t *testing.T) { } func TestMain(m *testing.M) { + flag.Parse() specutils.MaybeRunAsRoot() os.Exit(m.Run()) } diff --git a/runsc/cmd/install.go b/runsc/cmd/install.go new file mode 100644 index 000000000..441c1db0d --- /dev/null +++ b/runsc/cmd/install.go @@ -0,0 +1,210 @@ +// Copyright 2019 The gVisor Authors. +// +// 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 ( + "context" + "encoding/json" + "fmt" + "io/ioutil" + "log" + "os" + "path" + + "flag" + "github.com/google/subcommands" +) + +// Install implements subcommands.Command. +type Install struct { + ConfigFile string + Runtime string + Experimental bool +} + +// Name implements subcommands.Command.Name. +func (*Install) Name() string { + return "install" +} + +// Synopsis implements subcommands.Command.Synopsis. +func (*Install) Synopsis() string { + return "adds a runtime to docker daemon configuration" +} + +// Usage implements subcommands.Command.Usage. +func (*Install) Usage() string { + return `install [flags] [-- [args...]] -- if provided, args are passed to the runtime +` +} + +// SetFlags implements subcommands.Command.SetFlags. +func (i *Install) SetFlags(fs *flag.FlagSet) { + fs.StringVar(&i.ConfigFile, "config_file", "/etc/docker/daemon.json", "path to Docker daemon config file") + fs.StringVar(&i.Runtime, "runtime", "runsc", "runtime name") + fs.BoolVar(&i.Experimental, "experimental", false, "enable experimental features") +} + +// Execute implements subcommands.Command.Execute. +func (i *Install) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { + // Grab the name and arguments. + runtimeArgs := f.Args() + + // Extract the executable. + path, err := os.Executable() + if err != nil { + log.Fatalf("Error reading current exectuable: %v", err) + } + + // Load the configuration file. + c, err := readConfig(i.ConfigFile) + if err != nil { + log.Fatalf("Error reading config file %q: %v", i.ConfigFile, err) + } + + // Add the given runtime. + var rts map[string]interface{} + if i, ok := c["runtimes"]; ok { + rts = i.(map[string]interface{}) + } else { + rts = make(map[string]interface{}) + c["runtimes"] = rts + } + rts[i.Runtime] = struct { + Path string `json:"path,omitempty"` + RuntimeArgs []string `json:"runtimeArgs,omitempty"` + }{ + Path: path, + RuntimeArgs: runtimeArgs, + } + + // Set experimental if required. + if i.Experimental { + c["experimental"] = true + } + + // Write out the runtime. + if err := writeConfig(c, i.ConfigFile); err != nil { + log.Fatalf("Error writing config file %q: %v", i.ConfigFile, err) + } + + // Success. + log.Printf("Added runtime %q with arguments %v to %q.", i.Runtime, runtimeArgs, i.ConfigFile) + return subcommands.ExitSuccess +} + +// Uninstall implements subcommands.Command. +type Uninstall struct { + ConfigFile string + Runtime string +} + +// Name implements subcommands.Command.Name. +func (*Uninstall) Name() string { + return "uninstall" +} + +// Synopsis implements subcommands.Command.Synopsis. +func (*Uninstall) Synopsis() string { + return "removes a runtime from docker daemon configuration" +} + +// Usage implements subcommands.Command.Usage. +func (*Uninstall) Usage() string { + return `uninstall [flags] +` +} + +// SetFlags implements subcommands.Command.SetFlags. +func (u *Uninstall) SetFlags(fs *flag.FlagSet) { + fs.StringVar(&u.ConfigFile, "config_file", "/etc/docker/daemon.json", "path to Docker daemon config file") + fs.StringVar(&u.Runtime, "runtime", "runsc", "runtime name") +} + +// Execute implements subcommands.Command.Execute. +func (u *Uninstall) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { + log.Printf("Removing runtime %q from %q.", u.Runtime, u.ConfigFile) + + c, err := readConfig(u.ConfigFile) + if err != nil { + log.Fatalf("Error reading config file %q: %v", u.ConfigFile, err) + } + + var rts map[string]interface{} + if i, ok := c["runtimes"]; ok { + rts = i.(map[string]interface{}) + } else { + log.Fatalf("runtime %q not found", u.Runtime) + } + if _, ok := rts[u.Runtime]; !ok { + log.Fatalf("runtime %q not found", u.Runtime) + } + delete(rts, u.Runtime) + + if err := writeConfig(c, u.ConfigFile); err != nil { + log.Fatalf("Error writing config file %q: %v", u.ConfigFile, err) + } + return subcommands.ExitSuccess +} + +func readConfig(path string) (map[string]interface{}, error) { + // Read the configuration data. + configBytes, err := ioutil.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + return nil, err + } + + // Unmarshal the configuration. + c := make(map[string]interface{}) + if len(configBytes) > 0 { + if err := json.Unmarshal(configBytes, &c); err != nil { + return nil, err + } + } + + return c, nil +} + +func writeConfig(c map[string]interface{}, filename string) error { + // Marshal the configuration. + b, err := json.MarshalIndent(c, "", " ") + if err != nil { + return err + } + + // Copy the old configuration. + old, err := ioutil.ReadFile(filename) + if err != nil { + if !os.IsNotExist(err) { + return fmt.Errorf("error reading config file %q: %v", filename, err) + } + } else { + if err := ioutil.WriteFile(filename+"~", old, 0644); err != nil { + return fmt.Errorf("error backing up config file %q: %v", filename, err) + } + } + + // Make the necessary directories. + if err := os.MkdirAll(path.Dir(filename), 0755); err != nil { + return fmt.Errorf("error creating config directory for %q: %v", filename, err) + } + + // Write the new configuration. + if err := ioutil.WriteFile(filename, b, 0644); err != nil { + return fmt.Errorf("error writing config file %q: %v", filename, err) + } + + return nil +} -- cgit v1.2.3 From ac38a7ead0870118d27d570a8a98a90a7a225a12 Mon Sep 17 00:00:00 2001 From: Robert Tonic Date: Thu, 19 Sep 2019 12:37:15 -0400 Subject: Place the host UDS mounting behind --fsgofer-host-uds-allowed. This commit allows the use of the `--fsgofer-host-uds-allowed` flag to enable mounting sockets and add the appropriate seccomp filters. --- runsc/boot/config.go | 3 ++ runsc/cmd/gofer.go | 25 +++++++++++----- runsc/container/container.go | 5 ++++ runsc/fsgofer/filter/config.go | 23 ++++++++------- runsc/fsgofer/filter/filter.go | 12 ++++++++ runsc/fsgofer/fsgofer.go | 18 +++++++++--- runsc/main.go | 66 ++++++++++++++++++++++-------------------- 7 files changed, 98 insertions(+), 54 deletions(-) (limited to 'runsc/cmd') diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 7ae0dd05d..954ad2c2a 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -138,6 +138,9 @@ type Config struct { // Overlay is whether to wrap the root filesystem in an overlay. Overlay bool + // fsGoferHostUDSAllowed enables the gofer to mount a host UDS + FSGoferHostUDSAllowed bool + // Network indicates what type of network to use. Network NetworkType diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index 9faabf494..8e63c80e0 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -56,10 +56,11 @@ var goferCaps = &specs.LinuxCapabilities{ // Gofer implements subcommands.Command for the "gofer" command, which starts a // filesystem gofer. This command should not be called directly. type Gofer struct { - bundleDir string - ioFDs intFlags - applyCaps bool - setUpRoot bool + bundleDir string + ioFDs intFlags + applyCaps bool + hostUDSAllowed bool + setUpRoot bool panicOnWrite bool specFD int @@ -86,6 +87,7 @@ func (g *Gofer) SetFlags(f *flag.FlagSet) { f.StringVar(&g.bundleDir, "bundle", "", "path to the root of the bundle directory, defaults to the current directory") f.Var(&g.ioFDs, "io-fds", "list of FDs to connect 9P servers. They must follow this order: root first, then mounts as defined in the spec") f.BoolVar(&g.applyCaps, "apply-caps", true, "if true, apply capabilities to restrict what the Gofer process can do") + f.BoolVar(&g.hostUDSAllowed, "host-uds-allowed", false, "if true, allow the Gofer to mount a host UDS") f.BoolVar(&g.panicOnWrite, "panic-on-write", false, "if true, panics on attempts to write to RO mounts. RW mounts are unnaffected") f.BoolVar(&g.setUpRoot, "setup-root", true, "if true, set up an empty root for the process") f.IntVar(&g.specFD, "spec-fd", -1, "required fd with the container spec") @@ -180,8 +182,9 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) for _, m := range spec.Mounts { if specutils.Is9PMount(m) { cfg := fsgofer.Config{ - ROMount: isReadonlyMount(m.Options), - PanicOnWrite: g.panicOnWrite, + ROMount: isReadonlyMount(m.Options), + PanicOnWrite: g.panicOnWrite, + HostUDSAllowed: g.hostUDSAllowed, } ap, err := fsgofer.NewAttachPoint(m.Destination, cfg) if err != nil { @@ -200,8 +203,14 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) Fatalf("too many FDs passed for mounts. mounts: %d, FDs: %d", mountIdx, len(g.ioFDs)) } - if err := filter.Install(); err != nil { - Fatalf("installing seccomp filters: %v", err) + if g.hostUDSAllowed { + if err := filter.InstallUDS(); err != nil { + Fatalf("installing UDS seccomp filters: %v", err) + } + } else { + if err := filter.Install(); err != nil { + Fatalf("installing seccomp filters: %v", err) + } } runServers(ats, g.ioFDs) diff --git a/runsc/container/container.go b/runsc/container/container.go index bbb364214..ceadb38aa 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -941,6 +941,11 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *boot.Config, bund args = append(args, "--panic-on-write=true") } + // Add support for mounting host UDS in the gofer + if conf.FSGoferHostUDSAllowed { + args = append(args, "--host-uds-allowed=true") + } + // Open the spec file to donate to the sandbox. specFile, err := specutils.OpenSpec(bundleDir) if err != nil { diff --git a/runsc/fsgofer/filter/config.go b/runsc/fsgofer/filter/config.go index 73407383d..8989cdb2f 100644 --- a/runsc/fsgofer/filter/config.go +++ b/runsc/fsgofer/filter/config.go @@ -26,16 +26,6 @@ import ( // allowedSyscalls is the set of syscalls executed by the gofer. var allowedSyscalls = seccomp.SyscallRules{ syscall.SYS_ACCEPT: {}, - syscall.SYS_SOCKET: []seccomp.Rule{ - { - seccomp.AllowValue(syscall.AF_UNIX), - }, - }, - syscall.SYS_CONNECT: []seccomp.Rule{ - { - seccomp.AllowAny{}, - }, - }, syscall.SYS_ARCH_PRCTL: []seccomp.Rule{ {seccomp.AllowValue(linux.ARCH_GET_FS)}, {seccomp.AllowValue(linux.ARCH_SET_FS)}, @@ -194,3 +184,16 @@ var allowedSyscalls = seccomp.SyscallRules{ syscall.SYS_UTIMENSAT: {}, syscall.SYS_WRITE: {}, } + +var udsSyscalls = seccomp.SyscallRules{ + syscall.SYS_SOCKET: []seccomp.Rule{ + { + seccomp.AllowValue(syscall.AF_UNIX), + }, + }, + syscall.SYS_CONNECT: []seccomp.Rule{ + { + seccomp.AllowAny{}, + }, + }, +} diff --git a/runsc/fsgofer/filter/filter.go b/runsc/fsgofer/filter/filter.go index 65053415f..12ef19d18 100644 --- a/runsc/fsgofer/filter/filter.go +++ b/runsc/fsgofer/filter/filter.go @@ -31,3 +31,15 @@ func Install() error { return seccomp.Install(s) } + +// InstallUDS installs the standard Gofer seccomp filters along with filters +// allowing the gofer to connect to a host UDS. +func InstallUDS() error { + // Use the base syscall + s := allowedSyscalls + + // Add additional filters required for connecting to the host's sockets. + s.Merge(udsSyscalls) + + return seccomp.Install(s) +} diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index 89171c811..d9f3ba8d6 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -85,6 +85,9 @@ type Config struct { // PanicOnWrite panics on attempts to write to RO mounts. PanicOnWrite bool + + // HostUDS prevents + HostUDSAllowed bool } type attachPoint struct { @@ -128,12 +131,21 @@ func (a *attachPoint) Attach() (p9.File, error) { return nil, fmt.Errorf("stat file %q, err: %v", a.prefix, err) } + // Acquire the attach point lock + a.attachedMu.Lock() + defer a.attachedMu.Unlock() + // Hold the file descriptor we are converting into a p9.File var f *fd.FD // Apply the S_IFMT bitmask so we can detect file type appropriately switch fmtStat := stat.Mode & syscall.S_IFMT; { case fmtStat == syscall.S_IFSOCK: + // Check to see if the CLI option has been set to allow the UDS mount + if !a.conf.HostUDSAllowed { + return nil, fmt.Errorf("host UDS support is disabled") + } + // Attempt to open a connection. Bubble up the failures. f, err = fd.OpenUnix(a.prefix) if err != nil { @@ -144,7 +156,7 @@ func (a *attachPoint) Attach() (p9.File, error) { // Default to Read/Write permissions. mode := syscall.O_RDWR - // If the configuration is Read Only & the mount point is a directory, + // If the configuration is Read Only or the mount point is a directory, // set the mode to Read Only. if a.conf.ROMount || fmtStat == syscall.S_IFDIR { mode = syscall.O_RDONLY @@ -157,9 +169,7 @@ func (a *attachPoint) Attach() (p9.File, error) { } } - // Close the connection if the UDS is already attached. - a.attachedMu.Lock() - defer a.attachedMu.Unlock() + // Close the connection if already attached. if a.attached { f.Close() return nil, fmt.Errorf("attach point already attached, prefix: %s", a.prefix) diff --git a/runsc/main.go b/runsc/main.go index c61583441..5eba949f6 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -63,17 +63,18 @@ var ( straceLogSize = flag.Uint("strace-log-size", 1024, "default size (in bytes) to log data argument blobs") // Flags that control sandbox runtime behavior. - platformName = flag.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm") - network = flag.String("network", "sandbox", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") - gso = flag.Bool("gso", true, "enable generic segmenation offload") - fileAccess = flag.String("file-access", "exclusive", "specifies which filesystem to use for the root mount: exclusive (default), shared. Volume mounts are always shared.") - overlay = flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") - watchdogAction = flag.String("watchdog-action", "log", "sets what action the watchdog takes when triggered: log (default), panic.") - panicSignal = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") - profile = flag.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") - netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") - numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") - rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") + platformName = flag.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm") + network = flag.String("network", "sandbox", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") + gso = flag.Bool("gso", true, "enable generic segmenation offload") + fileAccess = flag.String("file-access", "exclusive", "specifies which filesystem to use for the root mount: exclusive (default), shared. Volume mounts are always shared.") + fsGoferHostUDSAllowed = flag.Bool("fsgofer-host-uds-allowed", false, "Allow the gofer to mount Unix Domain Sockets.") + overlay = flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") + watchdogAction = flag.String("watchdog-action", "log", "sets what action the watchdog takes when triggered: log (default), panic.") + panicSignal = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") + profile = flag.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") + netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") + numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") + rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") @@ -171,27 +172,28 @@ func main() { // Create a new Config from the flags. conf := &boot.Config{ - RootDir: *rootDir, - Debug: *debug, - LogFilename: *logFilename, - LogFormat: *logFormat, - DebugLog: *debugLog, - DebugLogFormat: *debugLogFormat, - FileAccess: fsAccess, - Overlay: *overlay, - Network: netType, - GSO: *gso, - LogPackets: *logPackets, - Platform: platformType, - Strace: *strace, - StraceLogSize: *straceLogSize, - WatchdogAction: wa, - PanicSignal: *panicSignal, - ProfileEnable: *profile, - EnableRaw: *netRaw, - NumNetworkChannels: *numNetworkChannels, - Rootless: *rootless, - AlsoLogToStderr: *alsoLogToStderr, + RootDir: *rootDir, + Debug: *debug, + LogFilename: *logFilename, + LogFormat: *logFormat, + DebugLog: *debugLog, + DebugLogFormat: *debugLogFormat, + FileAccess: fsAccess, + FSGoferHostUDSAllowed: *fsGoferHostUDSAllowed, + Overlay: *overlay, + Network: netType, + GSO: *gso, + LogPackets: *logPackets, + Platform: platformType, + Strace: *strace, + StraceLogSize: *straceLogSize, + WatchdogAction: wa, + PanicSignal: *panicSignal, + ProfileEnable: *profile, + EnableRaw: *netRaw, + NumNetworkChannels: *numNetworkChannels, + Rootless: *rootless, + AlsoLogToStderr: *alsoLogToStderr, TestOnlyAllowRunAsCurrentUserWithoutChroot: *testOnlyAllowRunAsCurrentUserWithoutChroot, } -- cgit v1.2.3 From 46beb919121f02d8bd110a54fb8f6de5dfd2891e Mon Sep 17 00:00:00 2001 From: Robert Tonic Date: Thu, 19 Sep 2019 17:10:50 -0400 Subject: Fix documentation, clean up seccomp filter installation, rename helpers. Filter installation has been streamlined and functions renamed. Documentation has been fixed to be standards compliant, and missing documentation added. gofmt has also been applied to modified files. --- pkg/fd/fd.go | 6 +++--- runsc/boot/config.go | 2 +- runsc/cmd/gofer.go | 12 +++++------- runsc/fsgofer/filter/filter.go | 19 ++++++------------- runsc/fsgofer/fsgofer.go | 21 +++++++++++---------- 5 files changed, 26 insertions(+), 34 deletions(-) (limited to 'runsc/cmd') diff --git a/pkg/fd/fd.go b/pkg/fd/fd.go index 7f1f9d984..24e959944 100644 --- a/pkg/fd/fd.go +++ b/pkg/fd/fd.go @@ -17,12 +17,12 @@ package fd import ( "fmt" + "gvisor.dev/gvisor/pkg/unet" "io" "os" "runtime" "sync/atomic" "syscall" - "gvisor.dev/gvisor/pkg/unet" ) // ReadWriter implements io.ReadWriter, io.ReaderAt, and io.WriterAt for fd. It @@ -186,8 +186,8 @@ func OpenAt(dir *FD, path string, flags int, mode uint32) (*FD, error) { return New(f), nil } -// OpenUnix Open a Unix Domain Socket and return the file descriptor for it. -func OpenUnix(path string) (*FD, error) { +// DialUnix connects to a Unix Domain Socket and return the file descriptor. +func DialUnix(path string) (*FD, error) { socket, err := unet.Connect(path, false) return New(socket.FD()), err } diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 954ad2c2a..f1adaba01 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -138,7 +138,7 @@ type Config struct { // Overlay is whether to wrap the root filesystem in an overlay. Overlay bool - // fsGoferHostUDSAllowed enables the gofer to mount a host UDS + // FSGoferHostUDSAllowed enables the gofer to mount a host UDS. FSGoferHostUDSAllowed bool // Network indicates what type of network to use. diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index 8e63c80e0..fa4f0034d 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -204,13 +204,11 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } if g.hostUDSAllowed { - if err := filter.InstallUDS(); err != nil { - Fatalf("installing UDS seccomp filters: %v", err) - } - } else { - if err := filter.Install(); err != nil { - Fatalf("installing seccomp filters: %v", err) - } + filter.InstallUDSFilters() + } + + if err := filter.Install(); err != nil { + Fatalf("installing seccomp filters: %v", err) } runServers(ats, g.ioFDs) diff --git a/runsc/fsgofer/filter/filter.go b/runsc/fsgofer/filter/filter.go index 12ef19d18..8d4ec9c24 100644 --- a/runsc/fsgofer/filter/filter.go +++ b/runsc/fsgofer/filter/filter.go @@ -23,23 +23,16 @@ import ( // Install installs seccomp filters. func Install() error { - s := allowedSyscalls - // Set of additional filters used by -race and -msan. Returns empty // when not enabled. - s.Merge(instrumentationFilters()) + allowedSyscalls.Merge(instrumentationFilters()) - return seccomp.Install(s) + return seccomp.Install(allowedSyscalls) } -// InstallUDS installs the standard Gofer seccomp filters along with filters -// allowing the gofer to connect to a host UDS. -func InstallUDS() error { - // Use the base syscall - s := allowedSyscalls - +// InstallUDSFilters installs the seccomp filters required to let the gofer connect +// to a host UDS. +func InstallUDSFilters() { // Add additional filters required for connecting to the host's sockets. - s.Merge(udsSyscalls) - - return seccomp.Install(s) + allowedSyscalls.Merge(udsSyscalls) } diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index d9f3ba8d6..357d712c6 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -21,6 +21,7 @@ package fsgofer import ( + "errors" "fmt" "io" "math" @@ -86,7 +87,7 @@ type Config struct { // PanicOnWrite panics on attempts to write to RO mounts. PanicOnWrite bool - // HostUDS prevents + // HostUDSAllowed signals whether the gofer can mount a host's UDS. HostUDSAllowed bool } @@ -131,23 +132,23 @@ func (a *attachPoint) Attach() (p9.File, error) { return nil, fmt.Errorf("stat file %q, err: %v", a.prefix, err) } - // Acquire the attach point lock + // Acquire the attach point lock. a.attachedMu.Lock() defer a.attachedMu.Unlock() - // Hold the file descriptor we are converting into a p9.File + // Hold the file descriptor we are converting into a p9.File. var f *fd.FD - // Apply the S_IFMT bitmask so we can detect file type appropriately - switch fmtStat := stat.Mode & syscall.S_IFMT; { - case fmtStat == syscall.S_IFSOCK: - // Check to see if the CLI option has been set to allow the UDS mount + // Apply the S_IFMT bitmask so we can detect file type appropriately. + switch fmtStat := stat.Mode & syscall.S_IFMT; fmtStat { + case syscall.S_IFSOCK: + // Check to see if the CLI option has been set to allow the UDS mount. if !a.conf.HostUDSAllowed { - return nil, fmt.Errorf("host UDS support is disabled") + return nil, errors.New("host UDS support is disabled") } // Attempt to open a connection. Bubble up the failures. - f, err = fd.OpenUnix(a.prefix) + f, err = fd.DialUnix(a.prefix) if err != nil { return nil, err } @@ -1058,7 +1059,7 @@ func (l *localFile) Flush() error { // Connect implements p9.File. func (l *localFile) Connect(p9.ConnectFlags) (*fd.FD, error) { - return fd.OpenUnix(l.hostPath) + return fd.DialUnix(l.hostPath) } // Close implements p9.File. -- cgit v1.2.3 From f2ea8e6b249d729d4616ee219c0472bfff93a575 Mon Sep 17 00:00:00 2001 From: Nicolas Lacasse Date: Mon, 23 Sep 2019 17:04:45 -0700 Subject: Always set HOME env var with `runsc exec`. We already do this for `runsc run`, but need to do the same for `runsc exec`. PiperOrigin-RevId: 270793459 --- runsc/boot/BUILD | 1 + runsc/boot/loader.go | 32 +++++++++++++++----------------- runsc/boot/user.go | 28 ++++++++++++++++++++++++++-- runsc/boot/user_test.go | 3 ++- runsc/cmd/exec.go | 1 + runsc/dockerutil/dockerutil.go | 8 ++++++++ test/e2e/exec_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 95 insertions(+), 20 deletions(-) (limited to 'runsc/cmd') diff --git a/runsc/boot/BUILD b/runsc/boot/BUILD index 588bb8851..54d1ab129 100644 --- a/runsc/boot/BUILD +++ b/runsc/boot/BUILD @@ -109,6 +109,7 @@ go_test( "//pkg/sentry/arch:registers_go_proto", "//pkg/sentry/context/contexttest", "//pkg/sentry/fs", + "//pkg/sentry/kernel/auth", "//pkg/unet", "//runsc/fsgofer", "@com_github_opencontainers_runtime-spec//specs-go:go_default_library", diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 823a34619..d824d7dc5 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -20,7 +20,6 @@ import ( mrand "math/rand" "os" "runtime" - "strings" "sync" "sync/atomic" "syscall" @@ -535,23 +534,12 @@ func (l *Loader) run() error { return err } - // Read /etc/passwd for the user's HOME directory and set the HOME - // environment variable as required by POSIX if it is not overridden by - // the user. - hasHomeEnvv := false - for _, envv := range l.rootProcArgs.Envv { - if strings.HasPrefix(envv, "HOME=") { - hasHomeEnvv = true - } - } - if !hasHomeEnvv { - homeDir, err := getExecUserHome(ctx, l.rootProcArgs.MountNamespace, uint32(l.rootProcArgs.Credentials.RealKUID)) - if err != nil { - return fmt.Errorf("error reading exec user: %v", err) - } - - l.rootProcArgs.Envv = append(l.rootProcArgs.Envv, "HOME="+homeDir) + // Add the HOME enviroment variable if it is not already set. + envv, err := maybeAddExecUserHome(ctx, l.rootProcArgs.MountNamespace, l.rootProcArgs.Credentials.RealKUID, l.rootProcArgs.Envv) + if err != nil { + return err } + l.rootProcArgs.Envv = envv // Create the root container init task. It will begin running // when the kernel is started. @@ -815,6 +803,16 @@ func (l *Loader) executeAsync(args *control.ExecArgs) (kernel.ThreadID, error) { }) defer args.MountNamespace.DecRef() + // Add the HOME enviroment varible if it is not already set. + root := args.MountNamespace.Root() + defer root.DecRef() + ctx := fs.WithRoot(l.k.SupervisorContext(), root) + envv, err := maybeAddExecUserHome(ctx, args.MountNamespace, args.KUID, args.Envv) + if err != nil { + return 0, err + } + args.Envv = envv + // Start the process. proc := control.Proc{Kernel: l.k} args.PIDNamespace = tg.PIDNamespace() diff --git a/runsc/boot/user.go b/runsc/boot/user.go index d1d423a5c..56cc12ee0 100644 --- a/runsc/boot/user.go +++ b/runsc/boot/user.go @@ -16,6 +16,7 @@ package boot import ( "bufio" + "fmt" "io" "strconv" "strings" @@ -23,6 +24,7 @@ import ( "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/sentry/context" "gvisor.dev/gvisor/pkg/sentry/fs" + "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/usermem" ) @@ -42,7 +44,7 @@ func (r *fileReader) Read(buf []byte) (int, error) { // getExecUserHome returns the home directory of the executing user read from // /etc/passwd as read from the container filesystem. -func getExecUserHome(ctx context.Context, rootMns *fs.MountNamespace, uid uint32) (string, error) { +func getExecUserHome(ctx context.Context, rootMns *fs.MountNamespace, uid auth.KUID) (string, error) { // The default user home directory to return if no user matching the user // if found in the /etc/passwd found in the image. const defaultHome = "/" @@ -82,7 +84,7 @@ func getExecUserHome(ctx context.Context, rootMns *fs.MountNamespace, uid uint32 File: f, } - homeDir, err := findHomeInPasswd(uid, r, defaultHome) + homeDir, err := findHomeInPasswd(uint32(uid), r, defaultHome) if err != nil { return "", err } @@ -90,6 +92,28 @@ func getExecUserHome(ctx context.Context, rootMns *fs.MountNamespace, uid uint32 return homeDir, nil } +// maybeAddExecUserHome returns a new slice with the HOME enviroment variable +// set if the slice does not already contain it, otherwise it returns the +// original slice unmodified. +func maybeAddExecUserHome(ctx context.Context, mns *fs.MountNamespace, uid auth.KUID, envv []string) ([]string, error) { + // Check if the envv already contains HOME. + for _, env := range envv { + if strings.HasPrefix(env, "HOME=") { + // We have it. Return the original slice unmodified. + return envv, nil + } + } + + // Read /etc/passwd for the user's HOME directory and set the HOME + // environment variable as required by POSIX if it is not overridden by + // the user. + homeDir, err := getExecUserHome(ctx, mns, uid) + if err != nil { + return nil, fmt.Errorf("error reading exec user: %v", err) + } + return append(envv, "HOME="+homeDir), nil +} + // findHomeInPasswd parses a passwd file and returns the given user's home // directory. This function does it's best to replicate the runc's behavior. func findHomeInPasswd(uid uint32, passwd io.Reader, defaultHome string) (string, error) { diff --git a/runsc/boot/user_test.go b/runsc/boot/user_test.go index 906baf3e5..9aee2ad07 100644 --- a/runsc/boot/user_test.go +++ b/runsc/boot/user_test.go @@ -25,6 +25,7 @@ import ( specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.dev/gvisor/pkg/sentry/context/contexttest" "gvisor.dev/gvisor/pkg/sentry/fs" + "gvisor.dev/gvisor/pkg/sentry/kernel/auth" ) func setupTempDir() (string, error) { @@ -68,7 +69,7 @@ func setupPasswd(contents string, perms os.FileMode) func() (string, error) { // TestGetExecUserHome tests the getExecUserHome function. func TestGetExecUserHome(t *testing.T) { tests := map[string]struct { - uid uint32 + uid auth.KUID createRoot func() (string, error) expected string }{ diff --git a/runsc/cmd/exec.go b/runsc/cmd/exec.go index e817eff77..bf1225e1c 100644 --- a/runsc/cmd/exec.go +++ b/runsc/cmd/exec.go @@ -127,6 +127,7 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) Fatalf("getting environment variables: %v", err) } } + if e.Capabilities == nil { // enableRaw is set to true to prevent the filtering out of // CAP_NET_RAW. This is the opposite of Create() because exec diff --git a/runsc/dockerutil/dockerutil.go b/runsc/dockerutil/dockerutil.go index c073d8f75..e37ec0ffd 100644 --- a/runsc/dockerutil/dockerutil.go +++ b/runsc/dockerutil/dockerutil.go @@ -287,6 +287,14 @@ func (d *Docker) Exec(args ...string) (string, error) { return do(a...) } +// ExecAsUser calls 'docker exec' as the given user with the arguments +// provided. +func (d *Docker) ExecAsUser(user string, args ...string) (string, error) { + a := []string{"exec", "--user", user, d.Name} + a = append(a, args...) + return do(a...) +} + // ExecWithTerminal calls 'docker exec -it' with the arguments provided and // attaches a pty to stdio. func (d *Docker) ExecWithTerminal(args ...string) (*exec.Cmd, *os.File, error) { diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go index 267679268..7238c2afe 100644 --- a/test/e2e/exec_test.go +++ b/test/e2e/exec_test.go @@ -177,3 +177,45 @@ func TestExecEnv(t *testing.T) { t.Errorf("wanted exec output to contain %q, got %q", want, got) } } + +// Test that exec always has HOME environment set, even when not set in run. +func TestExecEnvHasHome(t *testing.T) { + // Base alpine image does not have any environment variables set. + if err := dockerutil.Pull("alpine"); err != nil { + t.Fatalf("docker pull failed: %v", err) + } + d := dockerutil.MakeDocker("exec-env-test") + + // We will check that HOME is set for root user, and also for a new + // non-root user we will create. + newUID := 1234 + newHome := "/foo/bar" + + // Create a new user with a home directory, and then sleep. + script := fmt.Sprintf(` + mkdir -p -m 777 %s && \ + adduser foo -D -u %d -h %s && \ + sleep 1000`, newHome, newUID, newHome) + if err := d.Run("alpine", "/bin/sh", "-c", script); err != nil { + t.Fatalf("docker run failed: %v", err) + } + defer d.CleanUp() + + // Exec "echo $HOME", and expect to see "/root". + got, err := d.Exec("/bin/sh", "-c", "echo $HOME") + if err != nil { + t.Fatalf("docker exec failed: %v", err) + } + if want := "/root"; !strings.Contains(got, want) { + t.Errorf("wanted exec output to contain %q, got %q", want, got) + } + + // Execute the same as uid 123 and expect newHome. + got, err = d.ExecAsUser(strconv.Itoa(newUID), "/bin/sh", "-c", "echo $HOME") + if err != nil { + t.Fatalf("docker exec failed: %v", err) + } + if want := newHome; !strings.Contains(got, want) { + t.Errorf("wanted exec output to contain %q, got %q", want, got) + } +} -- cgit v1.2.3 From 7810b30983ec4d3a706df01163c29814cd21d6ca Mon Sep 17 00:00:00 2001 From: Robert Tonic Date: Tue, 24 Sep 2019 18:24:10 -0400 Subject: Refactor command line options and remove the allowed terminology for uds --- runsc/boot/config.go | 5 ++-- runsc/cmd/gofer.go | 18 ++++++------ runsc/container/container.go | 5 ---- runsc/fsgofer/fsgofer.go | 10 +++++-- runsc/main.go | 68 ++++++++++++++++++++++---------------------- 5 files changed, 52 insertions(+), 54 deletions(-) (limited to 'runsc/cmd') diff --git a/runsc/boot/config.go b/runsc/boot/config.go index f1adaba01..b76b0e574 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -138,8 +138,8 @@ type Config struct { // Overlay is whether to wrap the root filesystem in an overlay. Overlay bool - // FSGoferHostUDSAllowed enables the gofer to mount a host UDS. - FSGoferHostUDSAllowed bool + // FSGoferHostUDS enables the gofer to mount a host UDS. + FSGoferHostUDS bool // Network indicates what type of network to use. Network NetworkType @@ -217,6 +217,7 @@ func (c *Config) ToFlags() []string { "--debug-log-format=" + c.DebugLogFormat, "--file-access=" + c.FileAccess.String(), "--overlay=" + strconv.FormatBool(c.Overlay), + "--fsgofer-host-uds=" + strconv.FormatBool(c.FSGoferHostUDS), "--network=" + c.Network.String(), "--log-packets=" + strconv.FormatBool(c.LogPackets), "--platform=" + c.Platform, diff --git a/runsc/cmd/gofer.go b/runsc/cmd/gofer.go index fa4f0034d..fbd579fb8 100644 --- a/runsc/cmd/gofer.go +++ b/runsc/cmd/gofer.go @@ -56,11 +56,10 @@ var goferCaps = &specs.LinuxCapabilities{ // Gofer implements subcommands.Command for the "gofer" command, which starts a // filesystem gofer. This command should not be called directly. type Gofer struct { - bundleDir string - ioFDs intFlags - applyCaps bool - hostUDSAllowed bool - setUpRoot bool + bundleDir string + ioFDs intFlags + applyCaps bool + setUpRoot bool panicOnWrite bool specFD int @@ -87,7 +86,6 @@ func (g *Gofer) SetFlags(f *flag.FlagSet) { f.StringVar(&g.bundleDir, "bundle", "", "path to the root of the bundle directory, defaults to the current directory") f.Var(&g.ioFDs, "io-fds", "list of FDs to connect 9P servers. They must follow this order: root first, then mounts as defined in the spec") f.BoolVar(&g.applyCaps, "apply-caps", true, "if true, apply capabilities to restrict what the Gofer process can do") - f.BoolVar(&g.hostUDSAllowed, "host-uds-allowed", false, "if true, allow the Gofer to mount a host UDS") f.BoolVar(&g.panicOnWrite, "panic-on-write", false, "if true, panics on attempts to write to RO mounts. RW mounts are unnaffected") f.BoolVar(&g.setUpRoot, "setup-root", true, "if true, set up an empty root for the process") f.IntVar(&g.specFD, "spec-fd", -1, "required fd with the container spec") @@ -182,9 +180,9 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) for _, m := range spec.Mounts { if specutils.Is9PMount(m) { cfg := fsgofer.Config{ - ROMount: isReadonlyMount(m.Options), - PanicOnWrite: g.panicOnWrite, - HostUDSAllowed: g.hostUDSAllowed, + ROMount: isReadonlyMount(m.Options), + PanicOnWrite: g.panicOnWrite, + HostUDS: conf.FSGoferHostUDS, } ap, err := fsgofer.NewAttachPoint(m.Destination, cfg) if err != nil { @@ -203,7 +201,7 @@ func (g *Gofer) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) Fatalf("too many FDs passed for mounts. mounts: %d, FDs: %d", mountIdx, len(g.ioFDs)) } - if g.hostUDSAllowed { + if conf.FSGoferHostUDS { filter.InstallUDSFilters() } diff --git a/runsc/container/container.go b/runsc/container/container.go index ceadb38aa..bbb364214 100644 --- a/runsc/container/container.go +++ b/runsc/container/container.go @@ -941,11 +941,6 @@ func (c *Container) createGoferProcess(spec *specs.Spec, conf *boot.Config, bund args = append(args, "--panic-on-write=true") } - // Add support for mounting host UDS in the gofer - if conf.FSGoferHostUDSAllowed { - args = append(args, "--host-uds-allowed=true") - } - // Open the spec file to donate to the sandbox. specFile, err := specutils.OpenSpec(bundleDir) if err != nil { diff --git a/runsc/fsgofer/fsgofer.go b/runsc/fsgofer/fsgofer.go index 357d712c6..507d52b50 100644 --- a/runsc/fsgofer/fsgofer.go +++ b/runsc/fsgofer/fsgofer.go @@ -87,8 +87,8 @@ type Config struct { // PanicOnWrite panics on attempts to write to RO mounts. PanicOnWrite bool - // HostUDSAllowed signals whether the gofer can mount a host's UDS. - HostUDSAllowed bool + // HostUDS signals whether the gofer can mount a host's UDS. + HostUDS bool } type attachPoint struct { @@ -143,7 +143,7 @@ func (a *attachPoint) Attach() (p9.File, error) { switch fmtStat := stat.Mode & syscall.S_IFMT; fmtStat { case syscall.S_IFSOCK: // Check to see if the CLI option has been set to allow the UDS mount. - if !a.conf.HostUDSAllowed { + if !a.conf.HostUDS { return nil, errors.New("host UDS support is disabled") } @@ -1059,6 +1059,10 @@ func (l *localFile) Flush() error { // Connect implements p9.File. func (l *localFile) Connect(p9.ConnectFlags) (*fd.FD, error) { + // Check to see if the CLI option has been set to allow the UDS mount. + if !l.attachPoint.conf.HostUDS { + return nil, errors.New("host UDS support is disabled") + } return fd.DialUnix(l.hostPath) } diff --git a/runsc/main.go b/runsc/main.go index 5eba949f6..b788b1f76 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -63,18 +63,18 @@ var ( straceLogSize = flag.Uint("strace-log-size", 1024, "default size (in bytes) to log data argument blobs") // Flags that control sandbox runtime behavior. - platformName = flag.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm") - network = flag.String("network", "sandbox", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") - gso = flag.Bool("gso", true, "enable generic segmenation offload") - fileAccess = flag.String("file-access", "exclusive", "specifies which filesystem to use for the root mount: exclusive (default), shared. Volume mounts are always shared.") - fsGoferHostUDSAllowed = flag.Bool("fsgofer-host-uds-allowed", false, "Allow the gofer to mount Unix Domain Sockets.") - overlay = flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") - watchdogAction = flag.String("watchdog-action", "log", "sets what action the watchdog takes when triggered: log (default), panic.") - panicSignal = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") - profile = flag.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") - netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") - numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") - rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") + platformName = flag.String("platform", "ptrace", "specifies which platform to use: ptrace (default), kvm") + network = flag.String("network", "sandbox", "specifies which network to use: sandbox (default), host, none. Using network inside the sandbox is more secure because it's isolated from the host network.") + gso = flag.Bool("gso", true, "enable generic segmenation offload") + fileAccess = flag.String("file-access", "exclusive", "specifies which filesystem to use for the root mount: exclusive (default), shared. Volume mounts are always shared.") + fsGoferHostUDS = flag.Bool("fsgofer-host-uds", false, "Allow the gofer to mount Unix Domain Sockets.") + overlay = flag.Bool("overlay", false, "wrap filesystem mounts with writable overlay. All modifications are stored in memory inside the sandbox.") + watchdogAction = flag.String("watchdog-action", "log", "sets what action the watchdog takes when triggered: log (default), panic.") + panicSignal = flag.Int("panic-signal", -1, "register signal handling that panics. Usually set to SIGUSR2(12) to troubleshoot hangs. -1 disables it.") + profile = flag.Bool("profile", false, "prepares the sandbox to use Golang profiler. Note that enabling profiler loosens the seccomp protection added to the sandbox (DO NOT USE IN PRODUCTION).") + netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") + numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") + rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") @@ -172,28 +172,28 @@ func main() { // Create a new Config from the flags. conf := &boot.Config{ - RootDir: *rootDir, - Debug: *debug, - LogFilename: *logFilename, - LogFormat: *logFormat, - DebugLog: *debugLog, - DebugLogFormat: *debugLogFormat, - FileAccess: fsAccess, - FSGoferHostUDSAllowed: *fsGoferHostUDSAllowed, - Overlay: *overlay, - Network: netType, - GSO: *gso, - LogPackets: *logPackets, - Platform: platformType, - Strace: *strace, - StraceLogSize: *straceLogSize, - WatchdogAction: wa, - PanicSignal: *panicSignal, - ProfileEnable: *profile, - EnableRaw: *netRaw, - NumNetworkChannels: *numNetworkChannels, - Rootless: *rootless, - AlsoLogToStderr: *alsoLogToStderr, + RootDir: *rootDir, + Debug: *debug, + LogFilename: *logFilename, + LogFormat: *logFormat, + DebugLog: *debugLog, + DebugLogFormat: *debugLogFormat, + FileAccess: fsAccess, + FSGoferHostUDS: *fsGoferHostUDS, + Overlay: *overlay, + Network: netType, + GSO: *gso, + LogPackets: *logPackets, + Platform: platformType, + Strace: *strace, + StraceLogSize: *straceLogSize, + WatchdogAction: wa, + PanicSignal: *panicSignal, + ProfileEnable: *profile, + EnableRaw: *netRaw, + NumNetworkChannels: *numNetworkChannels, + Rootless: *rootless, + AlsoLogToStderr: *alsoLogToStderr, TestOnlyAllowRunAsCurrentUserWithoutChroot: *testOnlyAllowRunAsCurrentUserWithoutChroot, } -- cgit v1.2.3 From 0b02c3d5e5bae87f5cdbf4ae20dad8344bef32c2 Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Tue, 1 Oct 2019 11:48:24 -0700 Subject: Prevent CAP_NET_RAW from appearing in exec 'docker exec' was getting CAP_NET_RAW even when --net-raw=false because it was not filtered out from when copying container's capabilities. PiperOrigin-RevId: 272260451 --- runsc/cmd/exec.go | 52 ++++++++++++++--------------- runsc/cmd/exec_test.go | 4 +-- runsc/container/BUILD | 1 + runsc/container/container_test.go | 25 ++++++++++++++ runsc/container/test_app/test_app.go | 65 ++++++++++++++++++++++++++++++++++++ runsc/dockerutil/dockerutil.go | 9 ++++- runsc/specutils/BUILD | 1 + runsc/specutils/specutils.go | 10 ++++++ test/e2e/BUILD | 2 ++ test/e2e/exec_test.go | 65 +++++++++++++++++++++++++++--------- 10 files changed, 190 insertions(+), 44 deletions(-) (limited to 'runsc/cmd') diff --git a/runsc/cmd/exec.go b/runsc/cmd/exec.go index bf1225e1c..d1e99243b 100644 --- a/runsc/cmd/exec.go +++ b/runsc/cmd/exec.go @@ -105,11 +105,11 @@ func (ex *Exec) SetFlags(f *flag.FlagSet) { // Execute implements subcommands.Command.Execute. It starts a process in an // already created container. func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { - e, id, err := ex.parseArgs(f) + conf := args[0].(*boot.Config) + e, id, err := ex.parseArgs(f, conf.EnableRaw) if err != nil { Fatalf("parsing process spec: %v", err) } - conf := args[0].(*boot.Config) waitStatus := args[1].(*syscall.WaitStatus) c, err := container.Load(conf.RootDir, id) @@ -117,6 +117,9 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) Fatalf("loading sandbox: %v", err) } + log.Debugf("Exec arguments: %+v", e) + log.Debugf("Exec capablities: %+v", e.Capabilities) + // Replace empty settings with defaults from container. if e.WorkingDirectory == "" { e.WorkingDirectory = c.Spec.Process.Cwd @@ -129,14 +132,11 @@ func (ex *Exec) Execute(_ context.Context, f *flag.FlagSet, args ...interface{}) } if e.Capabilities == nil { - // enableRaw is set to true to prevent the filtering out of - // CAP_NET_RAW. This is the opposite of Create() because exec - // requires the capability to be set explicitly, while 'docker - // run' sets it by default. - e.Capabilities, err = specutils.Capabilities(true /* enableRaw */, c.Spec.Process.Capabilities) + e.Capabilities, err = specutils.Capabilities(conf.EnableRaw, c.Spec.Process.Capabilities) if err != nil { Fatalf("creating capabilities: %v", err) } + log.Infof("Using exec capabilities from container: %+v", e.Capabilities) } // containerd expects an actual process to represent the container being @@ -283,14 +283,14 @@ func (ex *Exec) execChildAndWait(waitStatus *syscall.WaitStatus) subcommands.Exi // parseArgs parses exec information from the command line or a JSON file // depending on whether the --process flag was used. Returns an ExecArgs and // the ID of the container to be used. -func (ex *Exec) parseArgs(f *flag.FlagSet) (*control.ExecArgs, string, error) { +func (ex *Exec) parseArgs(f *flag.FlagSet, enableRaw bool) (*control.ExecArgs, string, error) { if ex.processPath == "" { // Requires at least a container ID and command. if f.NArg() < 2 { f.Usage() return nil, "", fmt.Errorf("both a container-id and command are required") } - e, err := ex.argsFromCLI(f.Args()[1:]) + e, err := ex.argsFromCLI(f.Args()[1:], enableRaw) return e, f.Arg(0), err } // Requires only the container ID. @@ -298,11 +298,11 @@ func (ex *Exec) parseArgs(f *flag.FlagSet) (*control.ExecArgs, string, error) { f.Usage() return nil, "", fmt.Errorf("a container-id is required") } - e, err := ex.argsFromProcessFile() + e, err := ex.argsFromProcessFile(enableRaw) return e, f.Arg(0), err } -func (ex *Exec) argsFromCLI(argv []string) (*control.ExecArgs, error) { +func (ex *Exec) argsFromCLI(argv []string, enableRaw bool) (*control.ExecArgs, error) { extraKGIDs := make([]auth.KGID, 0, len(ex.extraKGIDs)) for _, s := range ex.extraKGIDs { kgid, err := strconv.Atoi(s) @@ -315,7 +315,7 @@ func (ex *Exec) argsFromCLI(argv []string) (*control.ExecArgs, error) { var caps *auth.TaskCapabilities if len(ex.caps) > 0 { var err error - caps, err = capabilities(ex.caps) + caps, err = capabilities(ex.caps, enableRaw) if err != nil { return nil, fmt.Errorf("capabilities error: %v", err) } @@ -333,7 +333,7 @@ func (ex *Exec) argsFromCLI(argv []string) (*control.ExecArgs, error) { }, nil } -func (ex *Exec) argsFromProcessFile() (*control.ExecArgs, error) { +func (ex *Exec) argsFromProcessFile(enableRaw bool) (*control.ExecArgs, error) { f, err := os.Open(ex.processPath) if err != nil { return nil, fmt.Errorf("error opening process file: %s, %v", ex.processPath, err) @@ -343,21 +343,21 @@ func (ex *Exec) argsFromProcessFile() (*control.ExecArgs, error) { if err := json.NewDecoder(f).Decode(&p); err != nil { return nil, fmt.Errorf("error parsing process file: %s, %v", ex.processPath, err) } - return argsFromProcess(&p) + return argsFromProcess(&p, enableRaw) } // argsFromProcess performs all the non-IO conversion from the Process struct // to ExecArgs. -func argsFromProcess(p *specs.Process) (*control.ExecArgs, error) { +func argsFromProcess(p *specs.Process, enableRaw bool) (*control.ExecArgs, error) { // Create capabilities. var caps *auth.TaskCapabilities if p.Capabilities != nil { var err error - // enableRaw is set to true to prevent the filtering out of - // CAP_NET_RAW. This is the opposite of Create() because exec - // requires the capability to be set explicitly, while 'docker - // run' sets it by default. - caps, err = specutils.Capabilities(true /* enableRaw */, p.Capabilities) + // Starting from Docker 19, capabilities are explicitly set for exec (instead + // of nil like before). So we can't distinguish 'exec' from + // 'exec --privileged', as both specify CAP_NET_RAW. Therefore, filter + // CAP_NET_RAW in the same way as container start. + caps, err = specutils.Capabilities(enableRaw, p.Capabilities) if err != nil { return nil, fmt.Errorf("error creating capabilities: %v", err) } @@ -410,7 +410,7 @@ func resolveEnvs(envs ...[]string) ([]string, error) { // capabilities takes a list of capabilities as strings and returns an // auth.TaskCapabilities struct with those capabilities in every capability set. // This mimics runc's behavior. -func capabilities(cs []string) (*auth.TaskCapabilities, error) { +func capabilities(cs []string, enableRaw bool) (*auth.TaskCapabilities, error) { var specCaps specs.LinuxCapabilities for _, cap := range cs { specCaps.Ambient = append(specCaps.Ambient, cap) @@ -419,11 +419,11 @@ func capabilities(cs []string) (*auth.TaskCapabilities, error) { specCaps.Inheritable = append(specCaps.Inheritable, cap) specCaps.Permitted = append(specCaps.Permitted, cap) } - // enableRaw is set to true to prevent the filtering out of - // CAP_NET_RAW. This is the opposite of Create() because exec requires - // the capability to be set explicitly, while 'docker run' sets it by - // default. - return specutils.Capabilities(true /* enableRaw */, &specCaps) + // Starting from Docker 19, capabilities are explicitly set for exec (instead + // of nil like before). So we can't distinguish 'exec' from + // 'exec --privileged', as both specify CAP_NET_RAW. Therefore, filter + // CAP_NET_RAW in the same way as container start. + return specutils.Capabilities(enableRaw, &specCaps) } // stringSlice allows a flag to be used multiple times, where each occurrence diff --git a/runsc/cmd/exec_test.go b/runsc/cmd/exec_test.go index eb38a431f..a1e980d08 100644 --- a/runsc/cmd/exec_test.go +++ b/runsc/cmd/exec_test.go @@ -91,7 +91,7 @@ func TestCLIArgs(t *testing.T) { } for _, tc := range testCases { - e, err := tc.ex.argsFromCLI(tc.argv) + e, err := tc.ex.argsFromCLI(tc.argv, true) if err != nil { t.Errorf("argsFromCLI(%+v): got error: %+v", tc.ex, err) } else if !cmp.Equal(*e, tc.expected, cmpopts.IgnoreUnexported(os.File{})) { @@ -144,7 +144,7 @@ func TestJSONArgs(t *testing.T) { } for _, tc := range testCases { - e, err := argsFromProcess(&tc.p) + e, err := argsFromProcess(&tc.p, true) if err != nil { t.Errorf("argsFromProcess(%+v): got error: %+v", tc.p, err) } else if !cmp.Equal(*e, tc.expected, cmpopts.IgnoreUnexported(os.File{})) { diff --git a/runsc/container/BUILD b/runsc/container/BUILD index bc1fa25e3..26d1cd5ab 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -47,6 +47,7 @@ go_test( ], deps = [ "//pkg/abi/linux", + "//pkg/bits", "//pkg/log", "//pkg/sentry/control", "//pkg/sentry/kernel", diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 2ac12e5b6..519f5ed9b 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -34,6 +34,7 @@ import ( "github.com/cenkalti/backoff" specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/bits" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/control" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" @@ -2049,6 +2050,30 @@ func TestMountSymlink(t *testing.T) { } } +// Check that --net-raw disables the CAP_NET_RAW capability. +func TestNetRaw(t *testing.T) { + capNetRaw := strconv.FormatUint(bits.MaskOf64(int(linux.CAP_NET_RAW)), 10) + app, err := testutil.FindFile("runsc/container/test_app/test_app") + if err != nil { + t.Fatal("error finding test_app:", err) + } + + for _, enableRaw := range []bool{true, false} { + conf := testutil.TestConfig() + conf.EnableRaw = enableRaw + + test := "--enabled" + if !enableRaw { + test = "--disabled" + } + + spec := testutil.NewSpecWithArgs(app, "capability", test, capNetRaw) + if err := run(spec, conf); err != nil { + t.Fatalf("Error running container: %v", err) + } + } +} + // executeSync synchronously executes a new process. func (cont *Container) executeSync(args *control.ExecArgs) (syscall.WaitStatus, error) { pid, err := cont.Execute(args) diff --git a/runsc/container/test_app/test_app.go b/runsc/container/test_app/test_app.go index 7f735c254..913d781c6 100644 --- a/runsc/container/test_app/test_app.go +++ b/runsc/container/test_app/test_app.go @@ -19,10 +19,12 @@ package main import ( "context" "fmt" + "io/ioutil" "log" "net" "os" "os/exec" + "regexp" "strconv" sys "syscall" "time" @@ -35,6 +37,7 @@ import ( func main() { subcommands.Register(subcommands.HelpCommand(), "") subcommands.Register(subcommands.FlagsCommand(), "") + subcommands.Register(new(capability), "") subcommands.Register(new(fdReceiver), "") subcommands.Register(new(fdSender), "") subcommands.Register(new(forkBomb), "") @@ -287,3 +290,65 @@ func (s *syscall) Execute(ctx context.Context, f *flag.FlagSet, args ...interfac } return subcommands.ExitSuccess } + +type capability struct { + enabled uint64 + disabled uint64 +} + +// Name implements subcommands.Command. +func (*capability) Name() string { + return "capability" +} + +// Synopsis implements subcommands.Command. +func (*capability) Synopsis() string { + return "checks if effective capabilities are set/unset" +} + +// Usage implements subcommands.Command. +func (*capability) Usage() string { + return "capability [--enabled=number] [--disabled=number]" +} + +// SetFlags implements subcommands.Command. +func (c *capability) SetFlags(f *flag.FlagSet) { + f.Uint64Var(&c.enabled, "enabled", 0, "") + f.Uint64Var(&c.disabled, "disabled", 0, "") +} + +// Execute implements subcommands.Command. +func (c *capability) Execute(ctx context.Context, f *flag.FlagSet, args ...interface{}) subcommands.ExitStatus { + if c.enabled == 0 && c.disabled == 0 { + fmt.Println("One of the flags must be set") + return subcommands.ExitUsageError + } + + status, err := ioutil.ReadFile("/proc/self/status") + if err != nil { + fmt.Printf("Error reading %q: %v\n", "proc/self/status", err) + return subcommands.ExitFailure + } + re := regexp.MustCompile("CapEff:\t([0-9a-f]+)\n") + matches := re.FindStringSubmatch(string(status)) + if matches == nil || len(matches) != 2 { + fmt.Printf("Effective capabilities not found in\n%s\n", status) + return subcommands.ExitFailure + } + caps, err := strconv.ParseUint(matches[1], 16, 64) + if err != nil { + fmt.Printf("failed to convert capabilities %q: %v\n", matches[1], err) + return subcommands.ExitFailure + } + + if c.enabled != 0 && (caps&c.enabled) != c.enabled { + fmt.Printf("Missing capabilities, want: %#x: got: %#x\n", c.enabled, caps) + return subcommands.ExitFailure + } + if c.disabled != 0 && (caps&c.disabled) != 0 { + fmt.Printf("Extra capabilities found, dont_want: %#x: got: %#x\n", c.disabled, caps) + return subcommands.ExitFailure + } + + return subcommands.ExitSuccess +} diff --git a/runsc/dockerutil/dockerutil.go b/runsc/dockerutil/dockerutil.go index e37ec0ffd..57f6ae8de 100644 --- a/runsc/dockerutil/dockerutil.go +++ b/runsc/dockerutil/dockerutil.go @@ -282,7 +282,14 @@ func (d *Docker) Logs() (string, error) { // Exec calls 'docker exec' with the arguments provided. func (d *Docker) Exec(args ...string) (string, error) { - a := []string{"exec", d.Name} + return d.ExecWithFlags(nil, args...) +} + +// ExecWithFlags calls 'docker exec name '. +func (d *Docker) ExecWithFlags(flags []string, args ...string) (string, error) { + a := []string{"exec"} + a = append(a, flags...) + a = append(a, d.Name) a = append(a, args...) return do(a...) } diff --git a/runsc/specutils/BUILD b/runsc/specutils/BUILD index fbfb8e2f8..fa58313a0 100644 --- a/runsc/specutils/BUILD +++ b/runsc/specutils/BUILD @@ -13,6 +13,7 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/abi/linux", + "//pkg/bits", "//pkg/log", "//pkg/sentry/kernel/auth", "@com_github_cenkalti_backoff//:go_default_library", diff --git a/runsc/specutils/specutils.go b/runsc/specutils/specutils.go index cb9e58dfb..591abe458 100644 --- a/runsc/specutils/specutils.go +++ b/runsc/specutils/specutils.go @@ -31,6 +31,7 @@ import ( "github.com/cenkalti/backoff" specs "github.com/opencontainers/runtime-spec/specs-go" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/bits" "gvisor.dev/gvisor/pkg/log" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" ) @@ -241,6 +242,15 @@ func AllCapabilities() *specs.LinuxCapabilities { } } +// AllCapabilitiesUint64 returns a bitmask containing all capabilities set. +func AllCapabilitiesUint64() uint64 { + var rv uint64 + for _, cap := range capFromName { + rv |= bits.MaskOf64(int(cap)) + } + return rv +} + var capFromName = map[string]linux.Capability{ "CAP_CHOWN": linux.CAP_CHOWN, "CAP_DAC_OVERRIDE": linux.CAP_DAC_OVERRIDE, diff --git a/test/e2e/BUILD b/test/e2e/BUILD index 99442cffb..4fe03a220 100644 --- a/test/e2e/BUILD +++ b/test/e2e/BUILD @@ -19,7 +19,9 @@ go_test( visibility = ["//:sandbox"], deps = [ "//pkg/abi/linux", + "//pkg/bits", "//runsc/dockerutil", + "//runsc/specutils", "//runsc/testutil", ], ) diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go index 7238c2afe..88d26e865 100644 --- a/test/e2e/exec_test.go +++ b/test/e2e/exec_test.go @@ -30,14 +30,17 @@ import ( "time" "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/bits" "gvisor.dev/gvisor/runsc/dockerutil" + "gvisor.dev/gvisor/runsc/specutils" ) +// Test that exec uses the exact same capability set as the container. func TestExecCapabilities(t *testing.T) { if err := dockerutil.Pull("alpine"); err != nil { t.Fatalf("docker pull failed: %v", err) } - d := dockerutil.MakeDocker("exec-test") + d := dockerutil.MakeDocker("exec-capabilities-test") // Start the container. if err := d.Run("alpine", "sh", "-c", "cat /proc/self/status; sleep 100"); err != nil { @@ -52,27 +55,59 @@ func TestExecCapabilities(t *testing.T) { if len(matches) != 2 { t.Fatalf("There should be a match for the whole line and the capability bitmask") } - capString := matches[1] - t.Log("Root capabilities:", capString) + want := fmt.Sprintf("CapEff:\t%s\n", matches[1]) + t.Log("Root capabilities:", want) - // CAP_NET_RAW was in the capability set for the container, but was - // removed. However, `exec` does not remove it. Verify that it's not - // set in the container, then re-add it for comparison. - caps, err := strconv.ParseUint(capString, 16, 64) + // Now check that exec'd process capabilities match the root. + got, err := d.Exec("grep", "CapEff:", "/proc/self/status") if err != nil { - t.Fatalf("failed to convert capabilities %q: %v", capString, err) + t.Fatalf("docker exec failed: %v", err) } - if caps&(1<