As a follow-up to #41919
Validation, and handling of --device-cgroup-rule could use some improvements.
1. Improve handling/validation of "a (all)" value
We currently treat a the same as other (c (char), b (block)) devices. However, the kernel documentation describes:
... A device cgroup associates a device access whitelist with each cgroup. A whitelist
entry has 4 fields. 'type' is a (all), c (char), or b (block). 'all' means it applies
to all types and all major and minor numbers. Major and minor are either an integer
or * for all. Access is a composition of r (read), w (write), and m (mknod).
In addition, that documentation shows an example where only the device-type is passed;
echo a > /sys/fs/cgroup/1/devices.allow
Related source code: https://github.com/torvalds/linux/blob/v5.10/security/device_cgroup.c#L614-L642
From the above;
- passing only
a should is allowed
- passing only
a is equivalent to "all" device-types, "all" major/minor numbers, and "all" permissions, so equivalent to a *:* rwm
- when passing
a in combination with major:minor and [rwm] permissions, both major/minor, and permissions are ignored
I think we should:
- consider allowing
a as valid value
- in that case, either;
- ignore
major:minor and [rwm, and warn that these fields are ignored (?)
- produce an error if the fields are not
*:* and rwm respectively (note: permissions can be ordered in any way, so rwm and mwr are equivalent)
2. Improve validation of major/minor
Looks like we currently don't validate against if the major/minor is in the correct range; https://github.com/torvalds/linux/blob/v5.10/include/linux/kdev_t.h
(12 bits are used to encode a major number (0..4095, it seems), 20 bits); https://stackoverflow.com/a/14849363
3. Validate on container "create" instead of on "start"
Validation doesn't take place until the container is started; we should validate the value when creating the container, so that we can fail early.
4. Update CLI accordingly, or remove client-side validation
We shouldn't validate this value on the client side, and just delegate validation to the daemon/API; if we do want to keep client-side validation, the rules should match what the daemon accepts (if we update).
As a follow-up to #41919
Validation, and handling of
--device-cgroup-rulecould use some improvements.1. Improve handling/validation of "a (all)" value
We currently treat
athe same as other (c (char),b (block)) devices. However, the kernel documentation describes:In addition, that documentation shows an example where only the device-type is passed;
echo a > /sys/fs/cgroup/1/devices.allowRelated source code: https://github.com/torvalds/linux/blob/v5.10/security/device_cgroup.c#L614-L642
From the above;
ashould is allowedais equivalent to "all" device-types, "all" major/minor numbers, and "all" permissions, so equivalent toa *:* rwmain combination withmajor:minorand[rwm]permissions, both major/minor, and permissions are ignoredI think we should:
aas valid valuemajor:minorand[rwm, and warn that these fields are ignored (?)*:*andrwmrespectively (note: permissions can be ordered in any way, sorwmandmwrare equivalent)2. Improve validation of major/minor
Looks like we currently don't validate against if the major/minor is in the correct range; https://github.com/torvalds/linux/blob/v5.10/include/linux/kdev_t.h
(12 bits are used to encode a major number (0..4095, it seems), 20 bits); https://stackoverflow.com/a/14849363
3. Validate on container "create" instead of on "start"
Validation doesn't take place until the container is started; we should validate the value when creating the container, so that we can fail early.
4. Update CLI accordingly, or remove client-side validation
We shouldn't validate this value on the client side, and just delegate validation to the daemon/API; if we do want to keep client-side validation, the rules should match what the daemon accepts (if we update).