cgroup2: devices: switch to emulator for cgroupv1 parity

There were several issues with the previous cgroupv2 devices filter
generator implementation, stemming from the previous implementation
using a few too many tricks to implement the correct cgroup behaviour
(rules were handled in reverse order, with wildcards having particularly
special interpretations). As a result, some slightly odd configurations
with rules in specific orders could result in incorrect filters being
generated.

By switching to the emulator which is already used by cgroupv1, we can
guarantee that the behaviour of filters in both cgroup versions will be
identical, as well as making use of the hardenings in the emulator (not
allowing users to add deny rules the kernel will ignore).

(Note that because the ordering of the devices emulator rules is
deterministic and based on the rule value, the existing test rules had
to be reordered slightly.)

Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This commit is contained in:
Aleksa Sarai
2021-05-12 15:56:14 +10:00
parent dcc1cf7c1c
commit 98a3c0e4db
3 changed files with 139 additions and 114 deletions
@@ -11,6 +11,7 @@ import (
"strconv"
"github.com/cilium/ebpf/asm"
devicesemulator "github.com/opencontainers/runc/libcontainer/cgroups/devices"
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/pkg/errors"
"golang.org/x/sys/unix"
@@ -22,11 +23,44 @@ const (
)
// DeviceFilter returns eBPF device filter program and its license string
func DeviceFilter(devices []*devices.Rule) (asm.Instructions, string, error) {
p := &program{}
func DeviceFilter(rules []*devices.Rule) (asm.Instructions, string, error) {
// Generate the minimum ruleset for the device rules we are given. While we
// don't care about minimum transitions in cgroupv2, using the emulator
// gives us a guarantee that the behaviour of devices filtering is the same
// as cgroupv1, including security hardenings to avoid misconfiguration
// (such as punching holes in wildcard rules).
emu := new(devicesemulator.Emulator)
for _, rule := range rules {
if err := emu.Apply(*rule); err != nil {
return nil, "", err
}
}
cleanRules, err := emu.Rules()
if err != nil {
return nil, "", err
}
p := &program{
defaultAllow: emu.IsBlacklist(),
}
p.init()
for i := len(devices) - 1; i >= 0; i-- {
if err := p.appendDevice(devices[i]); err != nil {
for idx, rule := range cleanRules {
if rule.Type == devices.WildcardDevice {
// We can safely skip over wildcard entries because there should
// only be one (at most) at the very start to instruct cgroupv1 to
// go into allow-list mode. However we do double-check this here.
if idx != 0 || rule.Allow != emu.IsBlacklist() {
return nil, "", errors.Errorf("[internal error] emulated cgroupv2 devices ruleset had bad wildcard at idx %v (%s)", idx, rule.CgroupString())
}
continue
}
if rule.Allow == p.defaultAllow {
// There should be no rules which have an action equal to the
// default action, the emulator removes those.
return nil, "", errors.Errorf("[internal error] emulated cgroupv2 devices ruleset had no-op rule at idx %v (%s)", idx, rule.CgroupString())
}
if err := p.appendRule(rule); err != nil {
return nil, "", err
}
}
@@ -35,9 +69,9 @@ func DeviceFilter(devices []*devices.Rule) (asm.Instructions, string, error) {
}
type program struct {
insts asm.Instructions
hasWildCard bool
blockID int
insts asm.Instructions
defaultAllow bool
blockID int
}
func (p *program) init() {
@@ -67,39 +101,36 @@ func (p *program) init() {
asm.LoadMem(asm.R5, asm.R1, 8, asm.Word))
}
// appendDevice needs to be called from the last element of OCI linux.resources.devices to the head element.
func (p *program) appendDevice(dev *devices.Rule) error {
// appendRule rule converts an OCI rule to the relevant eBPF block and adds it
// to the in-progress filter program. In order to operate properly, it must be
// called with a "clean" rule list (generated by devices.Emulator.Rules() --
// with any "a" rules removed).
func (p *program) appendRule(rule *devices.Rule) error {
if p.blockID < 0 {
return errors.New("the program is finalized")
}
if p.hasWildCard {
// All entries after wildcard entry are ignored
return nil
}
bpfType := int32(-1)
hasType := true
switch dev.Type {
case 'c':
switch rule.Type {
case devices.CharDevice:
bpfType = int32(unix.BPF_DEVCG_DEV_CHAR)
case 'b':
case devices.BlockDevice:
bpfType = int32(unix.BPF_DEVCG_DEV_BLOCK)
case 'a':
hasType = false
default:
// if not specified in OCI json, typ is set to DeviceTypeAll
return errors.Errorf("invalid Type %q", string(dev.Type))
// We do not permit 'a', nor any other types we don't know about.
return errors.Errorf("invalid Type %q", string(rule.Type))
}
if dev.Major > math.MaxUint32 {
return errors.Errorf("invalid major %d", dev.Major)
if rule.Major > math.MaxUint32 {
return errors.Errorf("invalid major %d", rule.Major)
}
if dev.Minor > math.MaxUint32 {
return errors.Errorf("invalid minor %d", dev.Major)
if rule.Minor > math.MaxUint32 {
return errors.Errorf("invalid minor %d", rule.Major)
}
hasMajor := dev.Major >= 0 // if not specified in OCI json, major is set to -1
hasMinor := dev.Minor >= 0
hasMajor := rule.Major >= 0 // if not specified in OCI json, major is set to -1
hasMinor := rule.Minor >= 0
bpfAccess := int32(0)
for _, r := range dev.Permissions {
for _, r := range rule.Permissions {
switch r {
case 'r':
bpfAccess |= unix.BPF_DEVCG_ACC_READ
@@ -136,19 +167,16 @@ func (p *program) appendDevice(dev *devices.Rule) error {
if hasMajor {
p.insts = append(p.insts,
// if (R4 != major) goto next
asm.JNE.Imm(asm.R4, int32(dev.Major), nextBlockSym),
asm.JNE.Imm(asm.R4, int32(rule.Major), nextBlockSym),
)
}
if hasMinor {
p.insts = append(p.insts,
// if (R5 != minor) goto next
asm.JNE.Imm(asm.R5, int32(dev.Minor), nextBlockSym),
asm.JNE.Imm(asm.R5, int32(rule.Minor), nextBlockSym),
)
}
if !hasType && !hasAccess && !hasMajor && !hasMinor {
p.hasWildCard = true
}
p.insts = append(p.insts, acceptBlock(dev.Allow)...)
p.insts = append(p.insts, acceptBlock(rule.Allow)...)
// set blockSym to the first instruction we added in this iteration
p.insts[prevBlockLastIdx+1] = p.insts[prevBlockLastIdx+1].Sym(blockSym)
p.blockID++
@@ -156,14 +184,14 @@ func (p *program) appendDevice(dev *devices.Rule) error {
}
func (p *program) finalize() (asm.Instructions, error) {
if p.hasWildCard {
// acceptBlock with asm.Return() is already inserted
return p.insts, nil
var v int32
if p.defaultAllow {
v = 1
}
blockSym := "block-" + strconv.Itoa(p.blockID)
p.insts = append(p.insts,
// R0 <- 0
asm.Mov.Imm32(asm.R0, 0).Sym(blockSym),
// R0 <- v
asm.Mov.Imm32(asm.R0, v).Sym(blockSym),
asm.Return(),
)
p.blockID = -1
@@ -171,7 +199,7 @@ func (p *program) finalize() (asm.Instructions, error) {
}
func acceptBlock(accept bool) asm.Instructions {
v := int32(0)
var v int32
if accept {
v = 1
}
@@ -55,81 +55,81 @@ block-0:
func TestDeviceFilter_BuiltInAllowList(t *testing.T) {
expected := `
// load parameters into registers
0: LdXMemW dst: r2 src: r1 off: 0 imm: 0
1: And32Imm dst: r2 imm: 65535
2: LdXMemW dst: r3 src: r1 off: 0 imm: 0
3: RSh32Imm dst: r3 imm: 16
4: LdXMemW dst: r4 src: r1 off: 4 imm: 0
5: LdXMemW dst: r5 src: r1 off: 8 imm: 0
0: LdXMemW dst: r2 src: r1 off: 0 imm: 0
1: And32Imm dst: r2 imm: 65535
2: LdXMemW dst: r3 src: r1 off: 0 imm: 0
3: RSh32Imm dst: r3 imm: 16
4: LdXMemW dst: r4 src: r1 off: 4 imm: 0
5: LdXMemW dst: r5 src: r1 off: 8 imm: 0
block-0:
// tuntap (c, 10, 200, rwm, allow)
6: JNEImm dst: r2 off: -1 imm: 2 <block-1>
7: JNEImm dst: r4 off: -1 imm: 10 <block-1>
8: JNEImm dst: r5 off: -1 imm: 200 <block-1>
9: Mov32Imm dst: r0 imm: 1
10: Exit
block-1:
11: JNEImm dst: r2 off: -1 imm: 2 <block-2>
12: JNEImm dst: r4 off: -1 imm: 5 <block-2>
13: JNEImm dst: r5 off: -1 imm: 2 <block-2>
14: Mov32Imm dst: r0 imm: 1
15: Exit
block-2:
// /dev/pts (c, 136, wildcard, rwm, true)
16: JNEImm dst: r2 off: -1 imm: 2 <block-3>
17: JNEImm dst: r4 off: -1 imm: 136 <block-3>
18: Mov32Imm dst: r0 imm: 1
19: Exit
block-3:
20: JNEImm dst: r2 off: -1 imm: 2 <block-4>
21: JNEImm dst: r4 off: -1 imm: 1 <block-4>
22: JNEImm dst: r5 off: -1 imm: 9 <block-4>
23: Mov32Imm dst: r0 imm: 1
24: Exit
block-4:
25: JNEImm dst: r2 off: -1 imm: 2 <block-5>
26: JNEImm dst: r4 off: -1 imm: 1 <block-5>
27: JNEImm dst: r5 off: -1 imm: 5 <block-5>
28: Mov32Imm dst: r0 imm: 1
29: Exit
block-5:
30: JNEImm dst: r2 off: -1 imm: 2 <block-6>
31: JNEImm dst: r4 off: -1 imm: 5 <block-6>
32: JNEImm dst: r5 off: -1 imm: 0 <block-6>
33: Mov32Imm dst: r0 imm: 1
34: Exit
block-6:
35: JNEImm dst: r2 off: -1 imm: 2 <block-7>
36: JNEImm dst: r4 off: -1 imm: 1 <block-7>
37: JNEImm dst: r5 off: -1 imm: 7 <block-7>
38: Mov32Imm dst: r0 imm: 1
39: Exit
block-7:
40: JNEImm dst: r2 off: -1 imm: 2 <block-8>
41: JNEImm dst: r4 off: -1 imm: 1 <block-8>
42: JNEImm dst: r5 off: -1 imm: 8 <block-8>
43: Mov32Imm dst: r0 imm: 1
44: Exit
block-8:
45: JNEImm dst: r2 off: -1 imm: 2 <block-9>
46: JNEImm dst: r4 off: -1 imm: 1 <block-9>
47: JNEImm dst: r5 off: -1 imm: 3 <block-9>
48: Mov32Imm dst: r0 imm: 1
49: Exit
block-9:
// (b, wildcard, wildcard, m, true)
50: JNEImm dst: r2 off: -1 imm: 1 <block-10>
51: Mov32Reg dst: r1 src: r3
52: And32Imm dst: r1 imm: 1
53: JNEReg dst: r1 off: -1 src: r3 <block-10>
54: Mov32Imm dst: r0 imm: 1
55: Exit
block-10:
6: JNEImm dst: r2 off: -1 imm: 1 <block-1>
7: Mov32Reg dst: r1 src: r3
8: And32Imm dst: r1 imm: 1
9: JNEReg dst: r1 off: -1 src: r3 <block-1>
10: Mov32Imm dst: r0 imm: 1
11: Exit
block-1:
// (c, wildcard, wildcard, m, true)
56: JNEImm dst: r2 off: -1 imm: 2 <block-11>
57: Mov32Reg dst: r1 src: r3
58: And32Imm dst: r1 imm: 1
59: JNEReg dst: r1 off: -1 src: r3 <block-11>
12: JNEImm dst: r2 off: -1 imm: 2 <block-2>
13: Mov32Reg dst: r1 src: r3
14: And32Imm dst: r1 imm: 1
15: JNEReg dst: r1 off: -1 src: r3 <block-2>
16: Mov32Imm dst: r0 imm: 1
17: Exit
block-2:
18: JNEImm dst: r2 off: -1 imm: 2 <block-3>
19: JNEImm dst: r4 off: -1 imm: 1 <block-3>
20: JNEImm dst: r5 off: -1 imm: 3 <block-3>
21: Mov32Imm dst: r0 imm: 1
22: Exit
block-3:
23: JNEImm dst: r2 off: -1 imm: 2 <block-4>
24: JNEImm dst: r4 off: -1 imm: 1 <block-4>
25: JNEImm dst: r5 off: -1 imm: 5 <block-4>
26: Mov32Imm dst: r0 imm: 1
27: Exit
block-4:
28: JNEImm dst: r2 off: -1 imm: 2 <block-5>
29: JNEImm dst: r4 off: -1 imm: 1 <block-5>
30: JNEImm dst: r5 off: -1 imm: 7 <block-5>
31: Mov32Imm dst: r0 imm: 1
32: Exit
block-5:
33: JNEImm dst: r2 off: -1 imm: 2 <block-6>
34: JNEImm dst: r4 off: -1 imm: 1 <block-6>
35: JNEImm dst: r5 off: -1 imm: 8 <block-6>
36: Mov32Imm dst: r0 imm: 1
37: Exit
block-6:
38: JNEImm dst: r2 off: -1 imm: 2 <block-7>
39: JNEImm dst: r4 off: -1 imm: 1 <block-7>
40: JNEImm dst: r5 off: -1 imm: 9 <block-7>
41: Mov32Imm dst: r0 imm: 1
42: Exit
block-7:
43: JNEImm dst: r2 off: -1 imm: 2 <block-8>
44: JNEImm dst: r4 off: -1 imm: 5 <block-8>
45: JNEImm dst: r5 off: -1 imm: 0 <block-8>
46: Mov32Imm dst: r0 imm: 1
47: Exit
block-8:
48: JNEImm dst: r2 off: -1 imm: 2 <block-9>
49: JNEImm dst: r4 off: -1 imm: 5 <block-9>
50: JNEImm dst: r5 off: -1 imm: 2 <block-9>
51: Mov32Imm dst: r0 imm: 1
52: Exit
block-9:
// tuntap (c, 10, 200, rwm, allow)
53: JNEImm dst: r2 off: -1 imm: 2 <block-10>
54: JNEImm dst: r4 off: -1 imm: 10 <block-10>
55: JNEImm dst: r5 off: -1 imm: 200 <block-10>
56: Mov32Imm dst: r0 imm: 1
57: Exit
block-10:
// /dev/pts (c, 136, wildcard, rwm, true)
58: JNEImm dst: r2 off: -1 imm: 2 <block-11>
59: JNEImm dst: r4 off: -1 imm: 136 <block-11>
60: Mov32Imm dst: r0 imm: 1
61: Exit
block-11:
-3
View File
@@ -58,9 +58,6 @@ func setDevices(dirPath string, r *configs.Resources) error {
if r.SkipDevices {
return nil
}
// XXX: This is currently a white-list (but all callers pass a blacklist of
// devices). This is bad for a whole variety of reasons, but will need
// to be fixed with co-ordinated effort with downstreams.
insts, license, err := devicefilter.DeviceFilter(r.Devices)
if err != nil {
return err