Skip to content

Commit c7d80c1

Browse files
committed
libct: warn on amb caps when inh not set
Fixing a long standing bug in github.com/syndtr/gocapability package (ignoring errors when setting ambient caps, see [1]) revealed that it's not possible to raise those ambient capabilities for which inheritable capabilities are not raised. In other words, "the Ambient vector cannot contain values not raised in the Inh vector" ([2]). The example spec in libct/specconv had a few ambient capabilities set but no inheritable ones. As a result, when capability package with fix from [1] is used, we get an error trying to start a container ("unable to apply caps: permission denied"). The only decent way to fix this is to ignore raised ambient capabilities for which inheritable capabilities are not raised (essentially mimicking the old behavior). Let's also add a warning about ignored capabilities. Fix the example spec (remove the ambient caps). This is in preparation to switch to github.com/kolyshkin/capability. [1]: kolyshkin/capability#3 [2]: https://pkg.go.dev/kernel.org/pub/linux/libs/security/libcap/cap#IAB.SetVector Signed-off-by: Kir Kolyshkin <[email protected]>
1 parent be44ac4 commit c7d80c1

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

libcontainer/capabilities/capabilities.go

+28-6
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
package capabilities
44

55
import (
6+
"slices"
67
"sort"
78
"strings"
89
"sync"
@@ -57,27 +58,48 @@ func New(capConfig *configs.Capabilities) (*Caps, error) {
5758

5859
cm := capMap()
5960
unknownCaps := make(map[string]struct{})
61+
ignoredCaps := make(map[string]struct{})
6062
// capSlice converts the slice of capability names in caps, to their numeric
6163
// equivalent, and returns them as a slice. Unknown or unavailable capabilities
6264
// are not returned, but appended to unknownCaps.
63-
capSlice := func(caps []string) []capability.Cap {
65+
//
66+
// If mustHave argument is not nil, caps that are not present in mustHave
67+
// are appended to ignoredCaps instead of the resulting slice.
68+
capSlice := func(caps []string, mustHave []capability.Cap) []capability.Cap {
6469
out := make([]capability.Cap, 0, len(caps))
6570
for _, c := range caps {
6671
if v, ok := cm[c]; !ok {
6772
unknownCaps[c] = struct{}{}
6873
} else {
74+
if mustHave != nil && !slices.Contains(mustHave, v) {
75+
ignoredCaps[c] = struct{}{}
76+
continue
77+
}
6978
out = append(out, v)
7079
}
7180
}
7281
return out
7382
}
83+
inheritable := capSlice(capConfig.Inheritable, nil)
84+
// Ambient vector can not contain values not raised in the Inheritable vector,
85+
// and errors setting ambient capabilities were previously ignored due to a bug
86+
// (see https://github.com/kolyshkin/capability/pull/3), so to maintain backward
87+
// compatibility we have to ignore those Ambient caps that are not also raised
88+
// in Inheritable (and issue a warning).
89+
ambient := capSlice(capConfig.Ambient, inheritable)
90+
if len(ignoredCaps) > 0 {
91+
logrus.Warn("unable to set Ambient capabilities which are not set in Inheritable; ignoring following Ambient capabilities: ", mapKeys(ignoredCaps))
92+
clear(ignoredCaps)
93+
}
94+
7495
c.caps = map[capability.CapType][]capability.Cap{
75-
capability.BOUNDING: capSlice(capConfig.Bounding),
76-
capability.EFFECTIVE: capSlice(capConfig.Effective),
77-
capability.INHERITABLE: capSlice(capConfig.Inheritable),
78-
capability.PERMITTED: capSlice(capConfig.Permitted),
79-
capability.AMBIENT: capSlice(capConfig.Ambient),
96+
capability.BOUNDING: capSlice(capConfig.Bounding, nil),
97+
capability.EFFECTIVE: capSlice(capConfig.Effective, nil),
98+
capability.INHERITABLE: inheritable,
99+
capability.PERMITTED: capSlice(capConfig.Permitted, nil),
100+
capability.AMBIENT: ambient,
80101
}
102+
81103
if c.pid, err = capability.NewPid2(0); err != nil {
82104
return nil, err
83105
}

tests/integration/capabilities.bats

+8
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,14 @@ function teardown() {
5454
[[ "${output}" == *"NoNewPrivs: 1"* ]]
5555
}
5656

57+
@test "runc run [ambient caps not set in inheritable result in a warning]" {
58+
update_config ' .process.capabilities.inheritable = ["CAP_KILL"]
59+
| .process.capabilities.ambient = ["CAP_KILL", "CAP_AUDIT_WRITE"]'
60+
runc run test_amb
61+
[ "$status" -eq 0 ]
62+
[[ "$output" == **"ignoring following Ambient capabilities: [CAP_AUDIT_WRITE]"* ]]
63+
}
64+
5765
@test "runc exec --cap" {
5866
update_config ' .process.args = ["/bin/sh"]
5967
| .process.capabilities = {}'

0 commit comments

Comments
 (0)