Skip to content

Commit

Permalink
libct: warn on amb caps when inh not set
Browse files Browse the repository at this point in the history
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 <kolyshkin@gmail.com>
  • Loading branch information
kolyshkin committed Aug 7, 2024
1 parent 937b6e5 commit 5cfb91d
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
34 changes: 28 additions & 6 deletions libcontainer/capabilities/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package capabilities

import (
"slices"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -57,27 +58,48 @@ func New(capConfig *configs.Capabilities) (*Caps, error) {

cm := capMap()
unknownCaps := make(map[string]struct{})
ignoredCaps := make(map[string]struct{})
// capSlice converts the slice of capability names in caps, to their numeric
// equivalent, and returns them as a slice. Unknown or unavailable capabilities
// are not returned, but appended to unknownCaps.
capSlice := func(caps []string) []capability.Cap {
//
// If mustHave argument is not nil, caps that are not present in mustHave
// are appended to ignoredCaps instead of the resulting slice.
capSlice := func(caps []string, mustHave []capability.Cap) []capability.Cap {
out := make([]capability.Cap, 0, len(caps))
for _, c := range caps {
if v, ok := cm[c]; !ok {
unknownCaps[c] = struct{}{}
} else {
if mustHave != nil && !slices.Contains(mustHave, v) {
ignoredCaps[c] = struct{}{}
continue
}
out = append(out, v)
}
}
return out
}
inheritable := capSlice(capConfig.Inheritable, nil)
// Ambient vector can not contain values not raised in the Inheritable vector,
// and errors setting ambient capabilities were previously ignored due to a bug
// (see https://github.com/kolyshkin/capability/pull/3), so to maintain backward
// compatibility we have to ignore those Ambient caps that are not also raised
// in Inheritable (and issue a warning).
ambient := capSlice(capConfig.Ambient, inheritable)
if len(ignoredCaps) > 0 {
logrus.Warn("unable to set Ambient capabilities which are not set in Inheritable; ignoring following Ambient capabilities: ", mapKeys(ignoredCaps))
clear(ignoredCaps)
}

c.caps = map[capability.CapType][]capability.Cap{
capability.BOUNDING: capSlice(capConfig.Bounding),
capability.EFFECTIVE: capSlice(capConfig.Effective),
capability.INHERITABLE: capSlice(capConfig.Inheritable),
capability.PERMITTED: capSlice(capConfig.Permitted),
capability.AMBIENT: capSlice(capConfig.Ambient),
capability.BOUNDING: capSlice(capConfig.Bounding, nil),
capability.EFFECTIVE: capSlice(capConfig.Effective, nil),
capability.INHERITABLE: inheritable,
capability.PERMITTED: capSlice(capConfig.Permitted, nil),
capability.AMBIENT: ambient,
}

if c.pid, err = capability.NewPid2(0); err != nil {
return nil, err
}
Expand Down
8 changes: 8 additions & 0 deletions tests/integration/capabilities.bats
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,14 @@ function teardown() {
[[ "${output}" == *"NoNewPrivs: 1"* ]]
}

@test "runc run [ambient caps not set in inheritable result in a warning]" {
update_config ' .process.capabilities.inheritable = ["CAP_KILL"]
| .process.capabilities.ambient = ["CAP_KILL", "CAP_AUDIT_WRITE"]'
runc run test_amb
[ "$status" -eq 0 ]
[[ "$output" == **"ignoring following Ambient capabilities: [CAP_AUDIT_WRITE]"* ]]
}

@test "runc exec --cap" {
update_config ' .process.args = ["/bin/sh"]
| .process.capabilities = {}'
Expand Down

0 comments on commit 5cfb91d

Please sign in to comment.