diff --git a/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go b/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go index 5a1cdd91..b593eb43 100644 --- a/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go +++ b/libcontainer/cgroups/ebpf/devicefilter/devicefilter.go @@ -22,7 +22,7 @@ const ( ) // DeviceFilter returns eBPF device filter program and its license string -func DeviceFilter(devices []*configs.Device) (asm.Instructions, string, error) { +func DeviceFilter(devices []*configs.DeviceRule) (asm.Instructions, string, error) { p := &program{} p.init() for i := len(devices) - 1; i >= 0; i-- { @@ -68,7 +68,7 @@ func (p *program) init() { } // appendDevice needs to be called from the last element of OCI linux.resources.devices to the head element. -func (p *program) appendDevice(dev *configs.Device) error { +func (p *program) appendDevice(dev *configs.DeviceRule) error { if p.blockID < 0 { return errors.New("the program is finalized") } diff --git a/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go b/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go index af432e52..d81e90f8 100644 --- a/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go +++ b/libcontainer/cgroups/ebpf/devicefilter/devicefilter_test.go @@ -20,7 +20,7 @@ func hash(s, comm string) string { return strings.Join(res, "\n") } -func testDeviceFilter(t testing.TB, devices []*configs.Device, expectedStr string) { +func testDeviceFilter(t testing.TB, devices []*configs.DeviceRule, expectedStr string) { insts, _, err := DeviceFilter(devices) if err != nil { t.Fatalf("%s: %v (devices: %+v)", t.Name(), err, devices) @@ -137,11 +137,15 @@ block-11: 62: Mov32Imm dst: r0 imm: 0 63: Exit ` - testDeviceFilter(t, specconv.AllowedDevices, expected) + var devices []*configs.DeviceRule + for _, device := range specconv.AllowedDevices { + devices = append(devices, &device.DeviceRule) + } + testDeviceFilter(t, devices, expected) } func TestDeviceFilter_Privileged(t *testing.T) { - devices := []*configs.Device{ + devices := []*configs.DeviceRule{ { Type: 'a', Major: -1, @@ -168,7 +172,7 @@ block-0: } func TestDeviceFilter_PrivilegedExceptSingleDevice(t *testing.T) { - devices := []*configs.Device{ + devices := []*configs.DeviceRule{ { Type: 'a', Major: -1, @@ -208,7 +212,7 @@ block-1: } func TestDeviceFilter_Weird(t *testing.T) { - devices := []*configs.Device{ + devices := []*configs.DeviceRule{ { Type: 'b', Major: 8, diff --git a/libcontainer/cgroups/fs/devices_test.go b/libcontainer/cgroups/fs/devices_test.go index 90b9f084..77e29124 100644 --- a/libcontainer/cgroups/fs/devices_test.go +++ b/libcontainer/cgroups/fs/devices_test.go @@ -18,14 +18,12 @@ func TestDevicesSetAllow(t *testing.T) { "devices.deny": "", }) - helper.CgroupData.config.Resources.Devices = []*configs.Device{ + helper.CgroupData.config.Resources.Devices = []*configs.DeviceRule{ { - Path: "/dev/zero", - Type: 'c', + Type: configs.CharDevice, Major: 1, Minor: 5, - Permissions: "rwm", - FileMode: 0666, + Permissions: configs.DevicePermissions("rwm"), Allow: true, }, } diff --git a/libcontainer/cgroups/fs2/devices.go b/libcontainer/cgroups/fs2/devices.go index 96d6daeb..ee180bda 100644 --- a/libcontainer/cgroups/fs2/devices.go +++ b/libcontainer/cgroups/fs2/devices.go @@ -10,12 +10,10 @@ import ( "golang.org/x/sys/unix" ) -func isRWM(cgroupPermissions string) bool { - r := false - w := false - m := false - for _, rn := range cgroupPermissions { - switch rn { +func isRWM(perms configs.DevicePermissions) bool { + var r, w, m bool + for _, perm := range perms { + switch perm { case 'r': r = true case 'w': diff --git a/libcontainer/configs/cgroup_linux.go b/libcontainer/configs/cgroup_linux.go index 1d2dca43..6ab03b76 100644 --- a/libcontainer/configs/cgroup_linux.go +++ b/libcontainer/configs/cgroup_linux.go @@ -42,7 +42,7 @@ type Cgroup struct { type Resources struct { // Devices is the set of access rules for devices in the container. - Devices []*Device `json:"devices"` + Devices []*DeviceRule `json:"devices"` // Memory limit (in bytes) Memory int64 `json:"memory"` diff --git a/libcontainer/configs/device.go b/libcontainer/configs/device.go index 8701bb21..24c5bbfa 100644 --- a/libcontainer/configs/device.go +++ b/libcontainer/configs/device.go @@ -1,8 +1,12 @@ package configs import ( + "errors" "fmt" "os" + "strconv" + + "golang.org/x/sys/unix" ) const ( @@ -12,21 +16,11 @@ const ( // TODO Windows: This can be factored out in the future type Device struct { - // Device type, block, char, etc. - Type rune `json:"type"` + DeviceRule // Path to the device. Path string `json:"path"` - // Major is the device's major number. - Major int64 `json:"major"` - - // Minor is the device's minor number. - Minor int64 `json:"minor"` - - // Cgroup permissions format, rwm. - Permissions string `json:"permissions"` - // FileMode permission bits for the device. FileMode os.FileMode `json:"file_mode"` @@ -35,23 +29,154 @@ type Device struct { // Gid of the device. Gid uint32 `json:"gid"` +} - // Write the file to the allowed list +// DevicePermissions is a cgroupv1-style string to represent device access. It +// has to be a string for backward compatibility reasons, hence why it has +// methods to do set operations. +type DevicePermissions string + +const ( + deviceRead uint = (1 << iota) + deviceWrite + deviceMknod +) + +func (p DevicePermissions) toSet() uint { + var set uint + for _, perm := range p { + switch perm { + case 'r': + set |= deviceRead + case 'w': + set |= deviceWrite + case 'm': + set |= deviceMknod + } + } + return set +} + +func fromSet(set uint) DevicePermissions { + var perm string + if set&deviceRead == deviceRead { + perm += "r" + } + if set&deviceWrite == deviceWrite { + perm += "w" + } + if set&deviceMknod == deviceMknod { + perm += "m" + } + return DevicePermissions(perm) +} + +// Union returns the union of the two sets of DevicePermissions. +func (p DevicePermissions) Union(o DevicePermissions) DevicePermissions { + lhs := p.toSet() + rhs := o.toSet() + return fromSet(lhs | rhs) +} + +// Difference returns the set difference of the two sets of DevicePermissions. +// In set notation, A.Difference(B) gives you A\B. +func (p DevicePermissions) Difference(o DevicePermissions) DevicePermissions { + lhs := p.toSet() + rhs := o.toSet() + return fromSet(lhs &^ rhs) +} + +// Intersection computes the intersection of the two sets of DevicePermissions. +func (p DevicePermissions) Intersection(o DevicePermissions) DevicePermissions { + lhs := p.toSet() + rhs := o.toSet() + return fromSet(lhs & rhs) +} + +// IsEmpty returns whether the set of permissions in a DevicePermissions is +// empty. +func (p DevicePermissions) IsEmpty() bool { + return p == DevicePermissions("") +} + +// IsValid returns whether the set of permissions is a subset of valid +// permissions (namely, {r,w,m}). +func (p DevicePermissions) IsValid() bool { + return p == fromSet(p.toSet()) +} + +type DeviceType rune + +const ( + WildcardDevice DeviceType = 'a' + BlockDevice DeviceType = 'b' + CharDevice DeviceType = 'c' // or 'u' + FifoDevice DeviceType = 'p' +) + +func (t DeviceType) IsValid() bool { + switch t { + case WildcardDevice, BlockDevice, CharDevice, FifoDevice: + return true + default: + return false + } +} + +func (t DeviceType) CanMknod() bool { + switch t { + case BlockDevice, CharDevice, FifoDevice: + return true + default: + return false + } +} + +func (t DeviceType) CanCgroup() bool { + switch t { + case WildcardDevice, BlockDevice, CharDevice: + return true + default: + return false + } +} + +type DeviceRule struct { + // Type of device ('c' for char, 'b' for block). If set to 'a', this rule + // acts as a wildcard and all fields other than Allow are ignored. + Type DeviceType `json:"type"` + + // Major is the device's major number. + Major int64 `json:"major"` + + // Minor is the device's minor number. + Minor int64 `json:"minor"` + + // Permissions is the set of permissions that this rule applies to (in the + // cgroupv1 format -- any combination of "rwm"). + Permissions DevicePermissions `json:"permissions"` + + // Allow specifies whether this rule is allowed. Allow bool `json:"allow"` } -func (d *Device) CgroupString() string { - return fmt.Sprintf("%c %s:%s %s", d.Type, deviceNumberString(d.Major), deviceNumberString(d.Minor), d.Permissions) -} - -func (d *Device) Mkdev() int { - return int((d.Major << 8) | (d.Minor & 0xff) | ((d.Minor & 0xfff00) << 12)) -} - -// deviceNumberString converts the device number to a string return result. -func deviceNumberString(number int64) string { - if number == Wildcard { - return "*" +func (d *DeviceRule) CgroupString() string { + var ( + major = strconv.FormatInt(d.Major, 10) + minor = strconv.FormatInt(d.Minor, 10) + ) + if d.Major == Wildcard { + major = "*" } - return fmt.Sprint(number) + if d.Minor == Wildcard { + minor = "*" + } + return fmt.Sprintf("%c %s:%s %s", d.Type, major, minor, d.Permissions) +} + +func (d *DeviceRule) Mkdev() (uint64, error) { + if d.Major == Wildcard || d.Minor == Wildcard { + return 0, errors.New("cannot mkdev() device with wildcards") + } + return unix.Mkdev(uint32(d.Major), uint32(d.Minor)), nil } diff --git a/libcontainer/devices/devices.go b/libcontainer/devices/devices.go index 5dabe06c..702f913e 100644 --- a/libcontainer/devices/devices.go +++ b/libcontainer/devices/devices.go @@ -31,33 +31,33 @@ func DeviceFromPath(path, permissions string) (*configs.Device, error) { } var ( + devType configs.DeviceType + mode = stat.Mode devNumber = uint64(stat.Rdev) major = unix.Major(devNumber) minor = unix.Minor(devNumber) ) - if major == 0 { - return nil, ErrNotADevice - } - - var ( - devType rune - mode = stat.Mode - ) switch { case mode&unix.S_IFBLK == unix.S_IFBLK: - devType = 'b' + devType = configs.BlockDevice case mode&unix.S_IFCHR == unix.S_IFCHR: - devType = 'c' + devType = configs.CharDevice + case mode&unix.S_IFIFO == unix.S_IFIFO: + devType = configs.FifoDevice + default: + return nil, ErrNotADevice } return &configs.Device{ - Type: devType, - Path: path, - Major: int64(major), - Minor: int64(minor), - Permissions: permissions, - FileMode: os.FileMode(mode), - Uid: stat.Uid, - Gid: stat.Gid, + DeviceRule: configs.DeviceRule{ + Type: devType, + Major: int64(major), + Minor: int64(minor), + Permissions: configs.DevicePermissions(permissions), + }, + Path: path, + FileMode: os.FileMode(mode), + Uid: stat.Uid, + Gid: stat.Gid, }, nil } diff --git a/libcontainer/integration/template_test.go b/libcontainer/integration/template_test.go index 3f3a0811..30503a98 100644 --- a/libcontainer/integration/template_test.go +++ b/libcontainer/integration/template_test.go @@ -21,6 +21,10 @@ const defaultMountFlags = unix.MS_NOEXEC | unix.MS_NOSUID | unix.MS_NODEV // it uses a network strategy of just setting a loopback interface // and the default setup for devices func newTemplateConfig(rootfs string) *configs.Config { + var allowedDevices []*configs.DeviceRule + for _, device := range specconv.AllowedDevices { + allowedDevices = append(allowedDevices, &device.DeviceRule) + } return &configs.Config{ Rootfs: rootfs, Capabilities: &configs.Capabilities{ @@ -116,7 +120,7 @@ func newTemplateConfig(rootfs string) *configs.Config { Path: "integration/test", Resources: &configs.Resources{ MemorySwappiness: nil, - Devices: specconv.AllowedDevices, + Devices: allowedDevices, }, }, MaskPaths: []string{ diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index 40a5686d..1052b965 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -631,16 +631,20 @@ func createDeviceNode(rootfs string, node *configs.Device, bind bool) error { func mknodDevice(dest string, node *configs.Device) error { fileMode := node.FileMode switch node.Type { - case 'c', 'u': - fileMode |= unix.S_IFCHR - case 'b': + case configs.BlockDevice: fileMode |= unix.S_IFBLK - case 'p': + case configs.CharDevice: + fileMode |= unix.S_IFCHR + case configs.FifoDevice: fileMode |= unix.S_IFIFO default: return fmt.Errorf("%c is not a valid device type for device %s", node.Type, node.Path) } - if err := unix.Mknod(dest, uint32(fileMode), node.Mkdev()); err != nil { + dev, err := node.Mkdev() + if err != nil { + return err + } + if err := unix.Mknod(dest, uint32(fileMode), int(dev)); err != nil { return err } return unix.Chown(dest, int(node.Uid), int(node.Gid)) diff --git a/libcontainer/specconv/spec_linux.go b/libcontainer/specconv/spec_linux.go index 881b42db..6df2e2af 100644 --- a/libcontainer/specconv/spec_linux.go +++ b/libcontainer/specconv/spec_linux.go @@ -66,92 +66,129 @@ var mountPropagationMapping = map[string]int{ var AllowedDevices = []*configs.Device{ // allow mknod for any device { - Type: 'c', - Major: wildcard, - Minor: wildcard, - Permissions: "m", - Allow: true, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: configs.Wildcard, + Minor: configs.Wildcard, + Permissions: "m", + Allow: true, + }, }, { - Type: 'b', - Major: wildcard, - Minor: wildcard, - Permissions: "m", - Allow: true, + DeviceRule: configs.DeviceRule{ + Type: configs.BlockDevice, + Major: configs.Wildcard, + Minor: configs.Wildcard, + Permissions: "m", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/null", - Major: 1, - Minor: 3, - Permissions: "rwm", - Allow: true, + Path: "/dev/null", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 1, + Minor: 3, + Permissions: "rwm", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/random", - Major: 1, - Minor: 8, - Permissions: "rwm", - Allow: true, + Path: "/dev/random", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 1, + Minor: 8, + Permissions: "rwm", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/full", - Major: 1, - Minor: 7, - Permissions: "rwm", - Allow: true, + Path: "/dev/full", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 1, + Minor: 7, + Permissions: "rwm", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/tty", - Major: 5, - Minor: 0, - Permissions: "rwm", - Allow: true, + Path: "/dev/tty", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 5, + Minor: 0, + Permissions: "rwm", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/zero", - Major: 1, - Minor: 5, - Permissions: "rwm", - Allow: true, + Path: "/dev/zero", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 1, + Minor: 5, + Permissions: "rwm", + Allow: true, + }, }, { - Type: 'c', - Path: "/dev/urandom", - Major: 1, - Minor: 9, - Permissions: "rwm", - Allow: true, + Path: "/dev/urandom", + FileMode: 0666, + Uid: 0, + Gid: 0, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 1, + Minor: 9, + Permissions: "rwm", + Allow: true, + }, }, // /dev/pts/ - pts namespaces are "coming soon" { - Path: "", - Type: 'c', - Major: 136, - Minor: wildcard, - Permissions: "rwm", - Allow: true, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 136, + Minor: configs.Wildcard, + Permissions: "rwm", + Allow: true, + }, }, { - Path: "", - Type: 'c', - Major: 5, - Minor: 2, - Permissions: "rwm", - Allow: true, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 5, + Minor: 2, + Permissions: "rwm", + Allow: true, + }, }, // tuntap { - Path: "", - Type: 'c', - Major: 10, - Minor: 200, - Permissions: "rwm", - Allow: true, + DeviceRule: configs.DeviceRule{ + Type: configs.CharDevice, + Major: 10, + Minor: 200, + Permissions: "rwm", + Allow: true, + }, }, } @@ -451,14 +488,13 @@ func CreateCgroupConfig(opts *CreateOpts) (*configs.Cgroup, error) { if err != nil { return nil, err } - dd := &configs.Device{ + c.Resources.Devices = append(c.Resources.Devices, &configs.DeviceRule{ Type: dt, Major: major, Minor: minor, - Permissions: d.Access, + Permissions: configs.DevicePermissions(d.Access), Allow: d.Allow, - } - c.Resources.Devices = append(c.Resources.Devices, dd) + }) } if r.Memory != nil { if r.Memory.Limit != nil { @@ -583,98 +619,48 @@ func CreateCgroupConfig(opts *CreateOpts) (*configs.Cgroup, error) { } } } - // append the default allowed devices to the end of the list - c.Resources.Devices = append(c.Resources.Devices, AllowedDevices...) + // Append the default allowed devices to the end of the list. + // XXX: Really this should be prefixed... + for _, device := range AllowedDevices { + c.Resources.Devices = append(c.Resources.Devices, &device.DeviceRule) + } return c, nil } -func stringToCgroupDeviceRune(s string) (rune, error) { +func stringToCgroupDeviceRune(s string) (configs.DeviceType, error) { switch s { case "a": - return 'a', nil + return configs.WildcardDevice, nil case "b": - return 'b', nil + return configs.BlockDevice, nil case "c": - return 'c', nil + return configs.CharDevice, nil default: return 0, fmt.Errorf("invalid cgroup device type %q", s) } } -func stringToDeviceRune(s string) (rune, error) { +func stringToDeviceRune(s string) (configs.DeviceType, error) { switch s { case "p": - return 'p', nil - case "u": - return 'u', nil + return configs.FifoDevice, nil + case "u", "c": + return configs.CharDevice, nil case "b": - return 'b', nil - case "c": - return 'c', nil + return configs.BlockDevice, nil default: return 0, fmt.Errorf("invalid device type %q", s) } } func createDevices(spec *specs.Spec, config *configs.Config) error { - // add whitelisted devices - config.Devices = []*configs.Device{ - { - Type: 'c', - Path: "/dev/null", - Major: 1, - Minor: 3, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, - { - Type: 'c', - Path: "/dev/random", - Major: 1, - Minor: 8, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, - { - Type: 'c', - Path: "/dev/full", - Major: 1, - Minor: 7, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, - { - Type: 'c', - Path: "/dev/tty", - Major: 5, - Minor: 0, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, - { - Type: 'c', - Path: "/dev/zero", - Major: 1, - Minor: 5, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, - { - Type: 'c', - Path: "/dev/urandom", - Major: 1, - Minor: 9, - FileMode: 0666, - Uid: 0, - Gid: 0, - }, + // Add default set of devices. + for _, device := range AllowedDevices { + if device.Path != "" { + config.Devices = append(config.Devices, device) + } } - // merge in additional devices from the spec + // Merge in additional devices from the spec. if spec.Linux != nil { for _, d := range spec.Linux.Devices { var uid, gid uint32 @@ -694,10 +680,12 @@ func createDevices(spec *specs.Spec, config *configs.Config) error { filemode = *d.FileMode } device := &configs.Device{ - Type: dt, + DeviceRule: configs.DeviceRule{ + Type: dt, + Major: d.Major, + Minor: d.Minor, + }, Path: d.Path, - Major: d.Major, - Minor: d.Minor, FileMode: filemode, Uid: uid, Gid: gid,