From db81c0b02f2f947ae837a3e16471a148a66436eb Mon Sep 17 00:00:00 2001 From: Fabricio Voznika Date: Mon, 27 Aug 2018 11:09:06 -0700 Subject: Put fsgofer inside chroot Now each container gets its own dedicated gofer that is chroot'd to the rootfs path. This is done to add an extra layer of security in case the gofer gets compromised. PiperOrigin-RevId: 210396476 Change-Id: Iba21360a59dfe90875d61000db103f8609157ca0 --- runsc/boot/controller.go | 6 +-- runsc/boot/fs.go | 6 +-- runsc/boot/loader.go | 20 +++++++-- runsc/boot/loader_test.go | 106 ++++++++++++++++------------------------------ 4 files changed, 58 insertions(+), 80 deletions(-) (limited to 'runsc/boot') diff --git a/runsc/boot/controller.go b/runsc/boot/controller.go index 2d6b507b3..fdb6be5b1 100644 --- a/runsc/boot/controller.go +++ b/runsc/boot/controller.go @@ -212,11 +212,11 @@ func (cm *containerManager) Start(args *StartArgs, _ *struct{}) error { if path.Clean(args.CID) != args.CID { return fmt.Errorf("container ID shouldn't contain directory traversals such as \"..\": %q", args.CID) } - if len(args.FilePayload.Files) != 1 { - return fmt.Errorf("start arguments must contain one file for the container root") + if len(args.FilePayload.Files) == 0 { + return fmt.Errorf("start arguments must contain at least one file for the container root") } - tgid, err := cm.l.startContainer(cm.l.k, args.Spec, args.Conf, args.CID, args.FilePayload.Files[0]) + tgid, err := cm.l.startContainer(cm.l.k, args.Spec, args.Conf, args.CID, args.FilePayload.Files) if err != nil { return err } diff --git a/runsc/boot/fs.go b/runsc/boot/fs.go index 6f5379a6d..20d0e42ef 100644 --- a/runsc/boot/fs.go +++ b/runsc/boot/fs.go @@ -510,8 +510,6 @@ func createRestoreEnvironment(spec *specs.Spec, conf *Config, fds *fdDispenser) MountSources: make(map[string][]fs.MountArgs), } - mounts := compileMounts(spec) - // Add root mount. fd := fds.remove() opts := p9MountOptions(conf, fd) @@ -528,8 +526,8 @@ func createRestoreEnvironment(spec *specs.Spec, conf *Config, fds *fdDispenser) } renv.MountSources[rootFsName] = append(renv.MountSources[rootFsName], rootMount) - // Add submounts - for _, m := range mounts { + // Add submounts. + for _, m := range compileMounts(spec) { if err := addRestoreMount(conf, renv, m, fds); err != nil { return nil, err } diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 0e94cf215..3963ed55d 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -23,6 +23,7 @@ import ( "runtime" "sync" "sync/atomic" + "syscall" gtime "time" specs "github.com/opencontainers/runtime-spec/specs-go" @@ -377,7 +378,7 @@ func (l *Loader) run() error { // startContainer starts a child container. It returns the thread group ID of // the newly created process. -func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config, cid string, file *os.File) (kernel.ThreadID, error) { +func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config, cid string, files []*os.File) (kernel.ThreadID, error) { // Create capabilities. caps, err := specutils.Capabilities(spec.Process.Capabilities) if err != nil { @@ -414,11 +415,23 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config if err != nil { return 0, fmt.Errorf("failed to create new process: %v", err) } + + // Can't take ownership away from os.File. dup them to get a new FDs. + var ioFDs []int + for _, f := range files { + fd, err := syscall.Dup(int(f.Fd())) + if err != nil { + return 0, fmt.Errorf("failed to dup file: %v", err) + } + f.Close() + ioFDs = append(ioFDs, fd) + } + err = setFileSystemForProcess( &procArgs, spec, conf, - []int{int(file.Fd())}, // ioFDs + ioFDs, false, creds, procArgs.Limits, @@ -453,8 +466,7 @@ func (l *Loader) startContainer(k *kernel.Kernel, spec *specs.Spec, conf *Config return tgid, nil } -// TODO: Per-container namespaces must be supported -// for -pid. +// TODO: Per-container namespaces must be supported for -pid. // waitContainer waits for the root process of a container to exit. func (l *Loader) waitContainer(cid string, waitStatus *uint32) error { diff --git a/runsc/boot/loader_test.go b/runsc/boot/loader_test.go index f2f690b5d..2396d52c8 100644 --- a/runsc/boot/loader_test.go +++ b/runsc/boot/loader_test.go @@ -16,7 +16,6 @@ package boot import ( "fmt" - "io/ioutil" "math/rand" "os" "reflect" @@ -36,6 +35,15 @@ func init() { rand.Seed(time.Now().UnixNano()) } +func testConfig() *Config { + return &Config{ + RootDir: "unused_root_dir", + Network: NetworkNone, + FileAccess: FileAccessDirect, + DisableSeccomp: true, + } +} + // testSpec returns a simple spec that can be used in tests. func testSpec() *specs.Spec { return &specs.Spec{ @@ -55,12 +63,7 @@ func createLoader() (*Loader, error) { if err != nil { return nil, err } - conf := &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessDirect, - DisableSeccomp: true, - } + conf := testConfig() spec := testSpec() return New(spec, conf, fd, nil, false) } @@ -152,18 +155,6 @@ func TestStartSignal(t *testing.T) { // Test that MountNamespace can be created with various specs. func TestCreateMountNamespace(t *testing.T) { - conf := &Config{ - RootDir: "unused_root_dir", - FileAccess: FileAccessDirect, - DisableSeccomp: true, - } - - testFile, err := ioutil.TempFile(os.TempDir(), "create-mount-namespace-") - if err != nil { - t.Fatalf("ioutil.TempFile() failed, err: %v", err) - } - defer os.RemoveAll(testFile.Name()) - testCases := []struct { name string // Spec that will be used to create the mount manager. Note @@ -234,8 +225,7 @@ func TestCreateMountNamespace(t *testing.T) { }, { Destination: "/foo/qux", - Source: testFile.Name(), - Type: "bind", + Type: "tmpfs", }, { // File mounts with the same prefix. @@ -284,8 +274,7 @@ func TestCreateMountNamespace(t *testing.T) { { // Mount with the same prefix. Destination: "/dev/fd-foo", - Source: testFile.Name(), - Type: "bind", + Type: "tmpfs", }, { // Unsupported fs type. @@ -298,8 +287,7 @@ func TestCreateMountNamespace(t *testing.T) { }, { Destination: "/dev/bar", - Source: testFile.Name(), - Type: "bind", + Type: "tmpfs", }, }, }, @@ -339,19 +327,22 @@ func TestCreateMountNamespace(t *testing.T) { } for _, tc := range testCases { - ctx := contexttest.Context(t) - mm, err := createMountNamespace(ctx, ctx, &tc.spec, conf, nil) - if err != nil { - t.Fatalf("createMountNamespace test case %q failed: %v", tc.name, err) - } - defer mm.DecRef() - root := mm.Root() - defer root.DecRef() - for _, p := range tc.expectedPaths { - if _, err := mm.FindInode(ctx, root, root, p, 0); err != nil { - t.Errorf("expected path %v to exist with spec %v, but got error %v", p, tc.spec, err) + t.Run(tc.name, func(t *testing.T) { + conf := testConfig() + ctx := contexttest.Context(t) + mm, err := createMountNamespace(ctx, ctx, &tc.spec, conf, nil) + if err != nil { + t.Fatalf("createMountNamespace test case %q failed: %v", tc.name, err) } - } + defer mm.DecRef() + root := mm.Root() + defer root.DecRef() + for _, p := range tc.expectedPaths { + if _, err := mm.FindInode(ctx, root, root, p, 0); err != nil { + t.Errorf("expected path %v to exist with spec %v, but got error %v", p, tc.spec, err) + } + } + }) } } @@ -361,7 +352,7 @@ func TestRestoreEnvironment(t *testing.T) { testCases := []struct { name string spec *specs.Spec - conf *Config + fileAccess FileAccessType ioFDs []int errorExpected bool expectedRenv fs.RestoreEnvironment @@ -384,12 +375,7 @@ func TestRestoreEnvironment(t *testing.T) { }, }, }, - conf: &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessProxy, - DisableSeccomp: true, - }, + fileAccess: FileAccessProxy, ioFDs: []int{0}, errorExpected: false, expectedRenv: fs.RestoreEnvironment{ @@ -444,12 +430,7 @@ func TestRestoreEnvironment(t *testing.T) { }, }, }, - conf: &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessProxy, - DisableSeccomp: true, - }, + fileAccess: FileAccessProxy, ioFDs: []int{0, 1}, errorExpected: false, expectedRenv: fs.RestoreEnvironment{ @@ -508,12 +489,7 @@ func TestRestoreEnvironment(t *testing.T) { }, }, }, - conf: &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessProxy, - DisableSeccomp: true, - }, + fileAccess: FileAccessProxy, ioFDs: []int{0}, errorExpected: false, expectedRenv: fs.RestoreEnvironment{ @@ -572,12 +548,7 @@ func TestRestoreEnvironment(t *testing.T) { }, }, }, - conf: &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessDirect, - DisableSeccomp: true, - }, + fileAccess: FileAccessDirect, ioFDs: []int{0, 1}, errorExpected: true, }, @@ -596,20 +567,17 @@ func TestRestoreEnvironment(t *testing.T) { }, }, }, - conf: &Config{ - RootDir: "unused_root_dir", - Network: NetworkNone, - FileAccess: FileAccessDirect, - DisableSeccomp: true, - }, + fileAccess: FileAccessDirect, ioFDs: []int{0}, errorExpected: true, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + conf := testConfig() + conf.FileAccess = tc.fileAccess fds := &fdDispenser{fds: tc.ioFDs} - actualRenv, err := createRestoreEnvironment(tc.spec, tc.conf, fds) + actualRenv, err := createRestoreEnvironment(tc.spec, conf, fds) if !tc.errorExpected && err != nil { t.Fatalf("could not create restore environment for test:%s", tc.name) } else if tc.errorExpected { -- cgit v1.2.3