From 559b36010710ebc872c5b2445971cefcac8f1bcf Mon Sep 17 00:00:00 2001 From: Elvis Pranskevichus Date: Thu, 23 Mar 2023 13:31:07 -0700 Subject: [PATCH 1/3] drivers/exec: Fix handling of capabilities for unprivileged tasks Currently, the `exec` driver is only setting the Bounding set, which is not sufficient to actually enable the requisite capabilities for the task process. In order for the capabilities to survive `execve` performed by libcontainer, the `Permitted`, `Inheritable`, and `Ambient` sets must also be set. Per CAPABILITIES (7): > Ambient: This is a set of capabilities that are preserved across an > execve(2) of a program that is not privileged. The ambient capability > set obeys the invariant that no capability can ever be ambient if it > is not both permitted and inheritable. Fixes: #16642 --- drivers/shared/executor/executor_linux.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/drivers/shared/executor/executor_linux.go b/drivers/shared/executor/executor_linux.go index 4ab8367be554..5516660cd184 100644 --- a/drivers/shared/executor/executor_linux.go +++ b/drivers/shared/executor/executor_linux.go @@ -526,8 +526,17 @@ func configureCapabilities(cfg *lconfigs.Config, command *ExecCommand) { } default: // otherwise apply the plugin + task capability configuration + // + // The capabilities must be set in the Ambient set as libcontainer + // performs `execve`` as an unprivileged user. Ambient also requires + // that capabilities are Permitted and Inheritable. Setting Effective + // is unnecessary, because we only need the capabilities to become + // effective _after_ execve, not before. cfg.Capabilities = &lconfigs.Capabilities{ - Bounding: command.Capabilities, + Bounding: command.Capabilities, + Permitted: command.Capabilities, + Inheritable: command.Capabilities, + Ambient: command.Capabilities, } } } From 8bf0677d042c675d3fc3bf852cc9aaaa7dffd304 Mon Sep 17 00:00:00 2001 From: Elvis Pranskevichus Date: Mon, 27 Mar 2023 09:40:32 -0700 Subject: [PATCH 2/3] Fix test --- drivers/shared/executor/executor_linux_test.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 10278578267e..63629fe1e0bb 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -634,11 +634,11 @@ func TestExecutor_Capabilities(t *testing.T) { { user: "nobody", caps: ` -CapInh: 0000000000000000 -CapPrm: 0000000000000000 -CapEff: 0000000000000000 +CapInh: 00000000a80405fb +CapPrm: 00000000a80405fb +CapEff: 00000000a80405fb CapBnd: 00000000a80405fb -CapAmb: 0000000000000000`, +CapAmb: 00000000a80405fb`, }, { user: "root", From d626fedcf236c0a8e69880fa448324330d757ce2 Mon Sep 17 00:00:00 2001 From: Elvis Pranskevichus Date: Mon, 27 Mar 2023 11:37:34 -0700 Subject: [PATCH 3/3] Expand the test and add a changelog entry --- .changelog/16643.txt | 3 ++ .../shared/executor/executor_linux_test.go | 35 +++++++++++++++---- 2 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 .changelog/16643.txt diff --git a/.changelog/16643.txt b/.changelog/16643.txt new file mode 100644 index 000000000000..c01a3e7895c5 --- /dev/null +++ b/.changelog/16643.txt @@ -0,0 +1,3 @@ +```release-note:bug +driver/exec: Fixed a bug where `cap_drop` and `cap_add` would not expand capabilities +``` diff --git a/drivers/shared/executor/executor_linux_test.go b/drivers/shared/executor/executor_linux_test.go index 63629fe1e0bb..be16435ad86b 100644 --- a/drivers/shared/executor/executor_linux_test.go +++ b/drivers/shared/executor/executor_linux_test.go @@ -628,12 +628,14 @@ func TestExecutor_Capabilities(t *testing.T) { testutil.ExecCompatible(t) cases := []struct { - user string - caps string + user string + capAdd []string + capDrop []string + capsExpected string }{ { user: "nobody", - caps: ` + capsExpected: ` CapInh: 00000000a80405fb CapPrm: 00000000a80405fb CapEff: 00000000a80405fb @@ -642,13 +644,24 @@ CapAmb: 00000000a80405fb`, }, { user: "root", - caps: ` + capsExpected: ` CapInh: 0000000000000000 CapPrm: 0000003fffffffff CapEff: 0000003fffffffff CapBnd: 0000003fffffffff CapAmb: 0000000000000000`, }, + { + user: "nobody", + capDrop: []string{"all"}, + capAdd: []string{"net_bind_service"}, + capsExpected: ` +CapInh: 0000000000000400 +CapPrm: 0000000000000400 +CapEff: 0000000000000400 +CapBnd: 0000000000000400 +CapAmb: 0000000000000400`, + }, } for _, c := range cases { @@ -662,7 +675,17 @@ CapAmb: 0000000000000000`, execCmd.ResourceLimits = true execCmd.Cmd = "/bin/bash" execCmd.Args = []string{"-c", "cat /proc/$$/status"} - execCmd.Capabilities = capabilities.NomadDefaults().Slice(true) + + capsBasis := capabilities.NomadDefaults() + capsAllowed := capsBasis.Slice(true) + if c.capDrop != nil || c.capAdd != nil { + calcCaps, err := capabilities.Calculate( + capsBasis, capsAllowed, c.capAdd, c.capDrop) + require.NoError(t, err) + execCmd.Capabilities = calcCaps + } else { + execCmd.Capabilities = capsAllowed + } executor := NewExecutorWithIsolation(testlog.HCLogger(t)) defer executor.Shutdown("SIGKILL", 0) @@ -690,7 +713,7 @@ CapAmb: 0000000000000000`, return s } - expected := canonical(c.caps) + expected := canonical(c.capsExpected) tu.WaitForResult(func() (bool, error) { output := canonical(testExecCmd.stdout.String()) if !strings.Contains(output, expected) {