Merge pull request #4367 from kolyshkin/ambient-caps

Don't set ambient caps without inheritable ones
opencontainers · Sep 26, 2024 · e1635d5 · e1635d5
2 parents 8e03054 + 7a44910
commit e1635d5
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 74 deletions.
diff --git a/exec.go b/exec.go
@@ -234,7 +234,12 @@ func getProcess(context *cli.Context, bundle string) (*specs.Process, error) {
 			p.Capabilities.Bounding = append(p.Capabilities.Bounding, c)
 			p.Capabilities.Effective = append(p.Capabilities.Effective, c)
 			p.Capabilities.Permitted = append(p.Capabilities.Permitted, c)
-			p.Capabilities.Ambient = append(p.Capabilities.Ambient, c)
+			// Since ambient capabilities can't be set without inherritable,
+			// and runc exec --cap don't set inheritable, let's only set
+			// ambient if we already have some inheritable bits set from spec.
+			if p.Capabilities.Inheritable != nil {
+				p.Capabilities.Ambient = append(p.Capabilities.Ambient, c)
+			}
 		}
 	}
 	// append the passed env variables

diff --git a/libcontainer/README.md b/libcontainer/README.md
@@ -59,66 +59,14 @@ config := &configs.Config{
 	Rootfs: "/your/path/to/rootfs",
 	Capabilities: &configs.Capabilities{
 		Bounding: []string{
-			"CAP_CHOWN",
-			"CAP_DAC_OVERRIDE",
-			"CAP_FSETID",
-			"CAP_FOWNER",
-			"CAP_MKNOD",
-			"CAP_NET_RAW",
-			"CAP_SETGID",
-			"CAP_SETUID",
-			"CAP_SETFCAP",
-			"CAP_SETPCAP",
-			"CAP_NET_BIND_SERVICE",
-			"CAP_SYS_CHROOT",
 			"CAP_KILL",
 			"CAP_AUDIT_WRITE",
 		},
 		Effective: []string{
-			"CAP_CHOWN",
-			"CAP_DAC_OVERRIDE",
-			"CAP_FSETID",
-			"CAP_FOWNER",
-			"CAP_MKNOD",
-			"CAP_NET_RAW",
-			"CAP_SETGID",
-			"CAP_SETUID",
-			"CAP_SETFCAP",
-			"CAP_SETPCAP",
-			"CAP_NET_BIND_SERVICE",
-			"CAP_SYS_CHROOT",
 			"CAP_KILL",
 			"CAP_AUDIT_WRITE",
 		},
 		Permitted: []string{
-			"CAP_CHOWN",
-			"CAP_DAC_OVERRIDE",
-			"CAP_FSETID",
-			"CAP_FOWNER",
-			"CAP_MKNOD",
-			"CAP_NET_RAW",
-			"CAP_SETGID",
-			"CAP_SETUID",
-			"CAP_SETFCAP",
-			"CAP_SETPCAP",
-			"CAP_NET_BIND_SERVICE",
-			"CAP_SYS_CHROOT",
-			"CAP_KILL",
-			"CAP_AUDIT_WRITE",
-		},
-		Ambient: []string{
-			"CAP_CHOWN",
-			"CAP_DAC_OVERRIDE",
-			"CAP_FSETID",
-			"CAP_FOWNER",
-			"CAP_MKNOD",
-			"CAP_NET_RAW",
-			"CAP_SETGID",
-			"CAP_SETUID",
-			"CAP_SETFCAP",
-			"CAP_SETPCAP",
-			"CAP_NET_BIND_SERVICE",
-			"CAP_SYS_CHROOT",
 			"CAP_KILL",
 			"CAP_AUDIT_WRITE",
 		},

diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go
@@ -75,22 +75,6 @@ func newTemplateConfig(t *testing.T, p *tParam) *configs.Config {
 				"CAP_KILL",
 				"CAP_AUDIT_WRITE",
 			},
-			Ambient: []string{
-				"CAP_CHOWN",
-				"CAP_DAC_OVERRIDE",
-				"CAP_FSETID",
-				"CAP_FOWNER",
-				"CAP_MKNOD",
-				"CAP_NET_RAW",
-				"CAP_SETGID",
-				"CAP_SETUID",
-				"CAP_SETFCAP",
-				"CAP_SETPCAP",
-				"CAP_NET_BIND_SERVICE",
-				"CAP_SYS_CHROOT",
-				"CAP_KILL",
-				"CAP_AUDIT_WRITE",
-			},
 			Effective: []string{
 				"CAP_CHOWN",
 				"CAP_DAC_OVERRIDE",

diff --git a/libcontainer/specconv/example.go b/libcontainer/specconv/example.go
@@ -41,11 +41,6 @@ func Example() *specs.Spec {
 					"CAP_KILL",
 					"CAP_NET_BIND_SERVICE",
 				},
-				Ambient: []string{
-					"CAP_AUDIT_WRITE",
-					"CAP_KILL",
-					"CAP_NET_BIND_SERVICE",
-				},
 				Effective: []string{
 					"CAP_AUDIT_WRITE",
 					"CAP_KILL",

diff --git a/tests/integration/capabilities.bats b/tests/integration/capabilities.bats
@@ -53,3 +53,66 @@ function teardown() {
 	[[ "${output}" == *"CapPrm:	0000000000200000"* ]]
 	[[ "${output}" == *"NoNewPrivs:	1"* ]]
 }
+
+@test "runc exec --cap" {
+	update_config '	  .process.args = ["/bin/sh"]
+			| .process.capabilities = {}'
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_exec_cap
+	[ "$status" -eq 0 ]
+
+	runc exec test_exec_cap cat /proc/self/status
+	[ "$status" -eq 0 ]
+	# Check no capabilities are set.
+	[[ "${output}" == *"CapInh:	0000000000000000"* ]]
+	[[ "${output}" == *"CapPrm:	0000000000000000"* ]]
+	[[ "${output}" == *"CapEff:	0000000000000000"* ]]
+	[[ "${output}" == *"CapBnd:	0000000000000000"* ]]
+	[[ "${output}" == *"CapAmb:	0000000000000000"* ]]
+
+	runc exec --cap CAP_KILL --cap CAP_AUDIT_WRITE test_exec_cap cat /proc/self/status
+	[ "$status" -eq 0 ]
+	# Check capabilities are added into bounding/effective/permitted only,
+	# but not to inheritable or ambient.
+	#
+	# CAP_KILL is 5, the bit mask is 0x20 (1 << 5).
+	# CAP_AUDIT_WRITE is 26, the bit mask is 0x20000000 (1 << 26).
+	[[ "${output}" == *"CapInh:	0000000000000000"* ]]
+	[[ "${output}" == *"CapPrm:	0000000020000020"* ]]
+	[[ "${output}" == *"CapEff:	0000000020000020"* ]]
+	[[ "${output}" == *"CapBnd:	0000000020000020"* ]]
+	[[ "${output}" == *"CapAmb:	0000000000000000"* ]]
+}
+
+@test "runc exec --cap [ambient is set from spec]" {
+	update_config '	  .process.args = ["/bin/sh"]
+			| .process.capabilities.inheritable = ["CAP_CHOWN", "CAP_SYSLOG"]
+			| .process.capabilities.permitted = ["CAP_KILL", "CAP_CHOWN"]
+			| .process.capabilities.effective = ["CAP_KILL"]
+			| .process.capabilities.bounding = ["CAP_KILL", "CAP_CHOWN", "CAP_SYSLOG"]
+			| .process.capabilities.ambient = ["CAP_CHOWN"]'
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_some_caps
+	[ "$status" -eq 0 ]
+
+	runc exec test_some_caps cat /proc/self/status
+	[ "$status" -eq 0 ]
+	# Check that capabilities are as set in spec.
+	#
+	# CAP_CHOWN is 0, the bit mask is 0x1 (1 << 0)
+	# CAP_KILL is 5, the bit mask is 0x20 (1 << 5).
+	# CAP_SYSLOG is 34, the bit mask is 0x400000000 (1 << 34).
+	[[ "${output}" == *"CapInh:	0000000400000001"* ]]
+	[[ "${output}" == *"CapPrm:	0000000000000021"* ]]
+	[[ "${output}" == *"CapEff:	0000000000000021"* ]]
+	[[ "${output}" == *"CapBnd:	0000000400000021"* ]]
+	[[ "${output}" == *"CapAmb:	0000000000000001"* ]]
+
+	# Check that if config.json has an inheritable capability set,
+	# runc exec --cap adds ambient capabilities.
+	runc exec --cap CAP_SYSLOG test_some_caps cat /proc/self/status
+	[ "$status" -eq 0 ]
+	[[ "${output}" == *"CapInh:	0000000400000001"* ]]
+	[[ "${output}" == *"CapPrm:	0000000400000021"* ]]
+	[[ "${output}" == *"CapEff:	0000000400000021"* ]]
+	[[ "${output}" == *"CapBnd:	0000000400000021"* ]]
+	[[ "${output}" == *"CapAmb:	0000000400000001"* ]]
+}