From 82611af92503e5255a3382b2ef44a9a69832c6b5 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 20 May 2019 13:00:33 -0400 Subject: [PATCH 1/7] drivers/exec: Restore 0.8 capabilities Nomad 0.9 incidentally set effective capabilities that is higher than what's expected of a `nobody` process, and what's set in 0.8. This change restores the capabilities to ones used in Nomad 0.9. --- drivers/exec/driver_test.go | 1 + drivers/shared/executor/executor_linux.go | 45 ++++++++++++----------- 2 files changed, 24 insertions(+), 22 deletions(-) diff --git a/drivers/exec/driver_test.go b/drivers/exec/driver_test.go index 2c0a565bcd3..6319ed69aa1 100644 --- a/drivers/exec/driver_test.go +++ b/drivers/exec/driver_test.go @@ -475,6 +475,7 @@ func TestExecDriver_DevicesAndMounts(t *testing.T) { task := &drivers.TaskConfig{ ID: uuid.Generate(), Name: "test", + User: "root", // need permission to read mounts paths Resources: testResources, StdoutPath: filepath.Join(tmpDir, "task-stdout"), StderrPath: filepath.Join(tmpDir, "task-stderr"), diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index cdaea5dd8f2..cf0aa8df870 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -45,26 +45,8 @@ var ( // ExecutorCgroupMeasuredCpuStats is the list of CPU stats captures by the executor ExecutorCgroupMeasuredCpuStats = []string{"System Mode", "User Mode", "Throttled Periods", "Throttled Time", "Percent"} - - // allCaps is all linux capabilities which is used to configure libcontainer - allCaps []string ) -// initialize the allCaps var with all capabilities available on the system -func init() { - last := capability.CAP_LAST_CAP - // workaround for RHEL6 which has no /proc/sys/kernel/cap_last_cap - if last == capability.Cap(63) { - last = capability.CAP_BLOCK_SUSPEND - } - for _, cap := range capability.List() { - if cap > last { - continue - } - allCaps = append(allCaps, fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String()))) - } -} - // LibcontainerExecutor implements an Executor with the runc/libcontainer api type LibcontainerExecutor struct { id string @@ -569,17 +551,36 @@ func (l *LibcontainerExecutor) handleExecWait(ch chan *waitResult, process *libc func configureCapabilities(cfg *lconfigs.Config, command *ExecCommand) error { // TODO: allow better control of these + // use capabilities list as prior to adopting libcontainer in 0.9 + allCaps := supportedCaps() cfg.Capabilities = &lconfigs.Capabilities{ Bounding: allCaps, - Permitted: allCaps, - Inheritable: allCaps, - Ambient: allCaps, - Effective: allCaps, + Permitted: nil, + Inheritable: nil, + Ambient: nil, + Effective: nil, } return nil } +// supportedCaps returns a list of all supported capabilities in kernel +func supportedCaps() []string { + allCaps := []string{} + last := capability.CAP_LAST_CAP + // workaround for RHEL6 which has no /proc/sys/kernel/cap_last_cap + if last == capability.Cap(63) { + last = capability.CAP_BLOCK_SUSPEND + } + for _, cap := range capability.List() { + if cap > last { + continue + } + allCaps = append(allCaps, fmt.Sprintf("CAP_%s", strings.ToUpper(cap.String()))) + } + return allCaps +} + // configureIsolation prepares the isolation primitives of the container. // The process runs in a container configured with the following: // From 67188714a376885b77e518ff942b869f33f77277 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Mon, 20 May 2019 15:30:07 -0400 Subject: [PATCH 2/7] fix --- .../shared/executor/executor_linux_test.go | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 9d2a1e41e1d..e68586db3c8 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -239,6 +239,43 @@ func TestExecutor_EscapeContainer(t *testing.T) { require.NoError(err) } +func TestExecutor_Capabilities(t *testing.T) { + t.Parallel() + require := require.New(t) + testutil.ExecCompatible(t) + + testExecCmd := testExecutorCommandWithChroot(t) + execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir + defer allocDir.Destroy() + + execCmd.ResourceLimits = true + execCmd.Cmd = "/bin/sh" + execCmd.Args = []string{"-c", "cat /proc/$$/cmdline"} + + executor := NewExecutorWithIsolation(testlog.HCLogger(t)) + defer executor.Shutdown("SIGKILL", 0) + + _, err := executor.Launch(execCmd) + require.NoError(err) + + ch := make(chan interface{}) + go func() { + executor.Wait(context.Background()) + close(ch) + }() + + select { + case <-ch: + // all good + case <-time.After(5 * time.Second): + require.Fail("timeout waiting for exec to shutdown") + } + + output := testExecCmd.stdout.String() + require.Empty(output) + +} + func TestExecutor_ClientCleanup(t *testing.T) { t.Parallel() testutil.ExecCompatible(t) From 3e1b13692980ffab82b8b7a39848aaa5050287b5 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 24 May 2019 11:30:16 -0400 Subject: [PATCH 3/7] tests: Fix binary dir permissions --- drivers/shared/executor/executor_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/shared/executor/executor_test.go b/drivers/shared/executor/executor_test.go index 65af774a533..ac8b9d03442 100644 --- a/drivers/shared/executor/executor_test.go +++ b/drivers/shared/executor/executor_test.go @@ -473,7 +473,7 @@ func setupRootfsBinary(t *testing.T, rootfs, path string) { t.Helper() dst := filepath.Join(rootfs, path) - err := os.MkdirAll(filepath.Dir(dst), 666) + err := os.MkdirAll(filepath.Dir(dst), 0755) require.NoError(t, err) src := filepath.Join( From 1a6454d2425a54dd7d43e41d1d879a918bf4c3a6 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 24 May 2019 14:03:26 -0400 Subject: [PATCH 4/7] special case root capabilities --- drivers/shared/executor/executor_linux.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index cf0aa8df870..14a406aae45 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -553,12 +553,20 @@ func configureCapabilities(cfg *lconfigs.Config, command *ExecCommand) error { // TODO: allow better control of these // use capabilities list as prior to adopting libcontainer in 0.9 allCaps := supportedCaps() - cfg.Capabilities = &lconfigs.Capabilities{ - Bounding: allCaps, - Permitted: nil, - Inheritable: nil, - Ambient: nil, - Effective: nil, + + // match capabilities used in Nomad 0.8 + if command.User == "root" { + cfg.Capabilities = &lconfigs.Capabilities{ + Bounding: allCaps, + Permitted: allCaps, + Effective: allCaps, + Ambient: nil, + Inheritable: nil, + } + } else { + cfg.Capabilities = &lconfigs.Capabilities{ + Bounding: allCaps, + } } return nil From e855738e0c5746f79587fdab17703a8c0e2072f0 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 24 May 2019 14:50:23 -0400 Subject: [PATCH 5/7] use /bin/bash --- drivers/shared/executor/executor_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index e68586db3c8..66f5124e21c 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -249,7 +249,7 @@ func TestExecutor_Capabilities(t *testing.T) { defer allocDir.Destroy() execCmd.ResourceLimits = true - execCmd.Cmd = "/bin/sh" + execCmd.Cmd = "/bin/bash" execCmd.Args = []string{"-c", "cat /proc/$$/cmdline"} executor := NewExecutorWithIsolation(testlog.HCLogger(t)) From a1414bd36081aad8d540d9c691e6def799c82788 Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 24 May 2019 15:06:50 -0500 Subject: [PATCH 6/7] Test for expected capabilities specifically --- .../shared/executor/executor_linux_test.go | 86 +++++++++++++------ 1 file changed, 62 insertions(+), 24 deletions(-) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 66f5124e21c..c3c93c463fe 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -44,6 +44,7 @@ func testExecutorCommandWithChroot(t *testing.T) *testExecCmd { "/etc/ld.so.cache": "/etc/ld.so.cache", "/etc/ld.so.conf": "/etc/ld.so.conf", "/etc/ld.so.conf.d": "/etc/ld.so.conf.d", + "/etc/passwd": "/etc/passwd", "/lib": "/lib", "/lib64": "/lib64", "/usr/lib": "/usr/lib", @@ -241,38 +242,75 @@ func TestExecutor_EscapeContainer(t *testing.T) { func TestExecutor_Capabilities(t *testing.T) { t.Parallel() - require := require.New(t) testutil.ExecCompatible(t) - testExecCmd := testExecutorCommandWithChroot(t) - execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir - defer allocDir.Destroy() + cases := []struct { + user string + caps string + }{ + { + user: "nobody", + caps: ` +CapInh: 0000000000000000 +CapPrm: 0000000000000000 +CapEff: 0000000000000000 +CapBnd: 0000003fffffffff +CapAmb: 0000000000000000`, + }, + { + user: "root", + caps: ` +CapInh: 0000000000000000 +CapPrm: 0000003fffffffff +CapEff: 0000003fffffffff +CapBnd: 0000003fffffffff +CapAmb: 0000000000000000`, + }, + } - execCmd.ResourceLimits = true - execCmd.Cmd = "/bin/bash" - execCmd.Args = []string{"-c", "cat /proc/$$/cmdline"} + for _, c := range cases { + t.Run(c.user, func(t *testing.T) { + require := require.New(t) - executor := NewExecutorWithIsolation(testlog.HCLogger(t)) - defer executor.Shutdown("SIGKILL", 0) + testExecCmd := testExecutorCommandWithChroot(t) + execCmd, allocDir := testExecCmd.command, testExecCmd.allocDir + defer allocDir.Destroy() - _, err := executor.Launch(execCmd) - require.NoError(err) + execCmd.User = c.user + execCmd.ResourceLimits = true + execCmd.Cmd = "/bin/bash" + execCmd.Args = []string{"-c", "cat /proc/$$/status"} - ch := make(chan interface{}) - go func() { - executor.Wait(context.Background()) - close(ch) - }() + executor := NewExecutorWithIsolation(testlog.HCLogger(t)) + defer executor.Shutdown("SIGKILL", 0) - select { - case <-ch: - // all good - case <-time.After(5 * time.Second): - require.Fail("timeout waiting for exec to shutdown") - } + _, err := executor.Launch(execCmd) + require.NoError(err) - output := testExecCmd.stdout.String() - require.Empty(output) + ch := make(chan interface{}) + go func() { + executor.Wait(context.Background()) + close(ch) + }() + + select { + case <-ch: + // all good + case <-time.After(5 * time.Second): + require.Fail("timeout waiting for exec to shutdown") + } + + expected := strings.TrimSpace(c.caps) + tu.WaitForResult(func() (bool, error) { + output := testExecCmd.stdout.String() + act := strings.TrimSpace(string(output)) + if strings.Contains(output, expected) { + return false, fmt.Errorf("capabilities didn't match: want\n%v\n; got:\n%v\n", expected, act) + } + return true, nil + }, func(err error) { require.NoError(err) }) + }) + } } From 6217d50803fea52d0395d8442cf8ce80963ba7ba Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Fri, 24 May 2019 21:30:45 -0500 Subject: [PATCH 7/7] Fix test comparisons --- .../shared/executor/executor_linux_test.go | 40 +++++++++++-------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index c3c93c463fe..ed87a1dc791 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "regexp" "strconv" "strings" "testing" @@ -151,7 +152,8 @@ usr/ /etc/: ld.so.cache ld.so.conf -ld.so.conf.d/` +ld.so.conf.d/ +passwd` tu.WaitForResult(func() (bool, error) { output := testExecCmd.stdout.String() act := strings.TrimSpace(string(output)) @@ -251,20 +253,20 @@ func TestExecutor_Capabilities(t *testing.T) { { user: "nobody", caps: ` -CapInh: 0000000000000000 -CapPrm: 0000000000000000 -CapEff: 0000000000000000 -CapBnd: 0000003fffffffff -CapAmb: 0000000000000000`, +CapInh: 0000000000000000 +CapPrm: 0000000000000000 +CapEff: 0000000000000000 +CapBnd: 0000003fffffffff +CapAmb: 0000000000000000`, }, { user: "root", caps: ` -CapInh: 0000000000000000 -CapPrm: 0000003fffffffff -CapEff: 0000003fffffffff -CapBnd: 0000003fffffffff -CapAmb: 0000000000000000`, +CapInh: 0000000000000000 +CapPrm: 0000003fffffffff +CapEff: 0000003fffffffff +CapBnd: 0000003fffffffff +CapAmb: 0000000000000000`, }, } @@ -300,12 +302,18 @@ CapAmb: 0000000000000000`, require.Fail("timeout waiting for exec to shutdown") } - expected := strings.TrimSpace(c.caps) + canonical := func(s string) string { + s = strings.TrimSpace(s) + s = regexp.MustCompile("[ \t]+").ReplaceAllString(s, " ") + s = regexp.MustCompile("[\n\r]+").ReplaceAllString(s, "\n") + return s + } + + expected := canonical(c.caps) tu.WaitForResult(func() (bool, error) { - output := testExecCmd.stdout.String() - act := strings.TrimSpace(string(output)) - if strings.Contains(output, expected) { - return false, fmt.Errorf("capabilities didn't match: want\n%v\n; got:\n%v\n", expected, act) + output := canonical(testExecCmd.stdout.String()) + if !strings.Contains(output, expected) { + return false, fmt.Errorf("capabilities didn't match: want\n%v\n; got:\n%v\n", expected, output) } return true, nil }, func(err error) { require.NoError(err) })