cgroup: allow fine-tuning of device ACL permissions

Adding audit points showed that we were granting too much privilege
to qemu; it should not need any mknod rights to recreate any
devices.  On the other hand, lxc should have all device privileges.
The solution is adding a flag parameter.

This also lets us restrict write access to read-only disks.

* src/util/cgroup.h (virCgroup*Device*): Adjust prototypes.
* src/util/cgroup.c (virCgroupAllowDevice)
(virCgroupAllowDeviceMajor, virCgroupAllowDevicePath)
(virCgroupDenyDevice, virCgroupDenyDeviceMajor)
(virCgroupDenyDevicePath): Add parameter.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Update clients.
* src/lxc/lxc_controller.c (lxcSetContainerResources): Likewise.
* src/qemu/qemu_cgroup.c: Likewise.
(qemuSetupDiskPathAllow): Also, honor read-only disks.
This commit is contained in:
Eric Blake 2011-03-08 20:13:18 -07:00
parent 48096a0064
commit 5564c57528
5 changed files with 88 additions and 38 deletions

View File

@ -1,5 +1,6 @@
/* /*
* Copyright (C) 2010 Red Hat, Inc. Copyright IBM Corp. 2008 * Copyright (C) 2010-2011 Red Hat, Inc.
* Copyright IBM Corp. 2008
* *
* lxc_controller.c: linux container process controller * lxc_controller.c: linux container process controller
* *
@ -168,7 +169,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
rc = virCgroupAllowDevice(cgroup, rc = virCgroupAllowDevice(cgroup,
dev->type, dev->type,
dev->major, dev->major,
dev->minor); dev->minor,
VIR_CGROUP_DEVICE_RWM);
if (rc != 0) { if (rc != 0) {
virReportSystemError(-rc, virReportSystemError(-rc,
_("Unable to allow device %c:%d:%d for domain %s"), _("Unable to allow device %c:%d:%d for domain %s"),
@ -177,7 +179,8 @@ static int lxcSetContainerResources(virDomainDefPtr def)
} }
} }
rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY); rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY,
VIR_CGROUP_DEVICE_RWM);
if (rc != 0) { if (rc != 0) {
virReportSystemError(-rc, virReportSystemError(-rc,
_("Unable to allow PYT devices for domain %s"), _("Unable to allow PYT devices for domain %s"),

View File

@ -56,7 +56,7 @@ int qemuCgroupControllerActive(struct qemud_driver *driver,
} }
static int static int
qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED, qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
const char *path, const char *path,
size_t depth ATTRIBUTE_UNUSED, size_t depth ATTRIBUTE_UNUSED,
void *opaque) void *opaque)
@ -65,8 +65,9 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int rc; int rc;
VIR_DEBUG("Process path %s for disk", path); VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */ rc = virCgroupAllowDevicePath(data->cgroup, path,
rc = virCgroupAllowDevicePath(data->cgroup, path); (disk->readonly ? VIR_CGROUP_DEVICE_READ
: VIR_CGROUP_DEVICE_RW));
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc); qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) { if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */ if (rc == -EACCES) { /* Get this for root squash NFS */
@ -106,8 +107,8 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int rc; int rc;
VIR_DEBUG("Process path %s for disk", path); VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */ rc = virCgroupDenyDevicePath(data->cgroup, path,
rc = virCgroupDenyDevicePath(data->cgroup, path); VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc); qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
if (rc < 0) { if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */ if (rc == -EACCES) { /* Get this for root squash NFS */
@ -150,7 +151,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path); VIR_DEBUG("Process path '%s' for disk", dev->source.data.file.path);
rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path); rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
dev->source.data.file.path, rc); dev->source.data.file.path, rc);
if (rc < 0) { if (rc < 0) {
@ -172,7 +174,8 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
int rc; int rc;
VIR_DEBUG("Process path '%s' for USB device", path); VIR_DEBUG("Process path '%s' for USB device", path);
rc = virCgroupAllowDevicePath(data->cgroup, path); rc = virCgroupAllowDevicePath(data->cgroup, path,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc); qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
if (rc < 0) { if (rc < 0) {
virReportSystemError(-rc, virReportSystemError(-rc,
@ -226,7 +229,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
goto cleanup; goto cleanup;
} }
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR); rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR, qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
"pty", rc == 0); "pty", rc == 0);
if (rc != 0) { if (rc != 0) {
@ -240,7 +244,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC && ((vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
driver->vncAllowHostAudio) || driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) { (vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR); rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR, qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
"sound", rc == 0); "sound", rc == 0);
if (rc != 0) { if (rc != 0) {
@ -251,8 +256,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
} }
for (i = 0; deviceACL[i] != NULL ; i++) { for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup, rc = virCgroupAllowDevicePath(cgroup, deviceACL[i],
deviceACL[i]); VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc); qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
if (rc < 0 && if (rc < 0 &&
rc != -ENOENT) { rc != -ENOENT) {

View File

@ -1962,7 +1962,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
vm->def->name); vm->def->name);
goto endjob; goto endjob;
} }
rc = virCgroupAllowDevicePath(cgroup, path); rc = virCgroupAllowDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RW);
qemuAuditCgroupPath(vm, cgroup, "allow", path, rc); qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
if (rc < 0) { if (rc < 0) {
virReportSystemError(-rc, virReportSystemError(-rc,
@ -2012,7 +2013,8 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
VIR_WARN("failed to restore save state label on %s", path); VIR_WARN("failed to restore save state label on %s", path);
if (cgroup != NULL) { if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path); rc = virCgroupDenyDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc); qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0) if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d", VIR_WARN("Unable to deny device %s for %s %d",
@ -2044,7 +2046,8 @@ endjob:
} }
if (cgroup != NULL) { if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path); rc = virCgroupDenyDevicePath(cgroup, path,
VIR_CGROUP_DEVICE_RWM);
qemuAuditCgroupPath(vm, cgroup, "deny", path, rc); qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
if (rc < 0) if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d", VIR_WARN("Unable to deny device %s for %s: %d",

View File

@ -1081,7 +1081,7 @@ int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb)
/** /**
* virCgroupDenyAllDevices: * virCgroupDenyAllDevices:
* *
* @group: The cgroup to deny devices for * @group: The cgroup to deny all permissions, for all devices
* *
* Returns: 0 on success * Returns: 0 on success
*/ */
@ -1100,15 +1100,20 @@ int virCgroupDenyAllDevices(virCgroupPtr group)
* @type: The device type (i.e., 'c' or 'b') * @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device * @major: The major number of the device
* @minor: The minor number of the device * @minor: The minor number of the device
* @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
* *
* Returns: 0 on success * Returns: 0 on success
*/ */
int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor) int virCgroupAllowDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{ {
int rc; int rc;
char *devstr = NULL; char *devstr = NULL;
if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) { if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM; rc = -ENOMEM;
goto out; goto out;
} }
@ -1129,15 +1134,20 @@ out:
* @group: The cgroup to allow an entire device major type for * @group: The cgroup to allow an entire device major type for
* @type: The device type (i.e., 'c' or 'b') * @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type * @major: The major number of the device type
* @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
* *
* Returns: 0 on success * Returns: 0 on success
*/ */
int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major) int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major,
int perms)
{ {
int rc; int rc;
char *devstr = NULL; char *devstr = NULL;
if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) { if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM; rc = -ENOMEM;
goto out; goto out;
} }
@ -1157,6 +1167,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
* *
* @group: The cgroup to allow the device for * @group: The cgroup to allow the device for
* @path: the device to allow * @path: the device to allow
* @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to allow
* *
* Queries the type of device and its major/minor number, and * Queries the type of device and its major/minor number, and
* adds that to the cgroup ACL * adds that to the cgroup ACL
@ -1165,7 +1176,7 @@ int virCgroupAllowDeviceMajor(virCgroupPtr group, char type, int major)
* negative errno value on failure * negative errno value on failure
*/ */
#if defined(major) && defined(minor) #if defined(major) && defined(minor)
int virCgroupAllowDevicePath(virCgroupPtr group, const char *path) int virCgroupAllowDevicePath(virCgroupPtr group, const char *path, int perms)
{ {
struct stat sb; struct stat sb;
@ -1178,11 +1189,13 @@ int virCgroupAllowDevicePath(virCgroupPtr group, const char *path)
return virCgroupAllowDevice(group, return virCgroupAllowDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b', S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev), major(sb.st_rdev),
minor(sb.st_rdev)); minor(sb.st_rdev),
perms);
} }
#else #else
int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
const char *path ATTRIBUTE_UNUSED) const char *path ATTRIBUTE_UNUSED,
int perms ATTRIBUTE_UNUSED)
{ {
return -ENOSYS; return -ENOSYS;
} }
@ -1196,15 +1209,20 @@ int virCgroupAllowDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
* @type: The device type (i.e., 'c' or 'b') * @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device * @major: The major number of the device
* @minor: The minor number of the device * @minor: The minor number of the device
* @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
* *
* Returns: 0 on success * Returns: 0 on success
*/ */
int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor) int virCgroupDenyDevice(virCgroupPtr group, char type, int major, int minor,
int perms)
{ {
int rc; int rc;
char *devstr = NULL; char *devstr = NULL;
if (virAsprintf(&devstr, "%c %i:%i rwm", type, major, minor) == -1) { if (virAsprintf(&devstr, "%c %i:%i %s%s%s", type, major, minor,
perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM; rc = -ENOMEM;
goto out; goto out;
} }
@ -1225,15 +1243,20 @@ out:
* @group: The cgroup to deny an entire device major type for * @group: The cgroup to deny an entire device major type for
* @type: The device type (i.e., 'c' or 'b') * @type: The device type (i.e., 'c' or 'b')
* @major: The major number of the device type * @major: The major number of the device type
* @perms: Bitwise or of VIR_CGROUP_DEVICE permission bits to deny
* *
* Returns: 0 on success * Returns: 0 on success
*/ */
int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major) int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major,
int perms)
{ {
int rc; int rc;
char *devstr = NULL; char *devstr = NULL;
if (virAsprintf(&devstr, "%c %i:* rwm", type, major) == -1) { if (virAsprintf(&devstr, "%c %i:* %s%s%s", type, major,
perms & VIR_CGROUP_DEVICE_READ ? "r" : "",
perms & VIR_CGROUP_DEVICE_WRITE ? "w" : "",
perms & VIR_CGROUP_DEVICE_MKNOD ? "m" : "") == -1) {
rc = -ENOMEM; rc = -ENOMEM;
goto out; goto out;
} }
@ -1249,7 +1272,7 @@ int virCgroupDenyDeviceMajor(virCgroupPtr group, char type, int major)
} }
#if defined(major) && defined(minor) #if defined(major) && defined(minor)
int virCgroupDenyDevicePath(virCgroupPtr group, const char *path) int virCgroupDenyDevicePath(virCgroupPtr group, const char *path, int perms)
{ {
struct stat sb; struct stat sb;
@ -1262,11 +1285,13 @@ int virCgroupDenyDevicePath(virCgroupPtr group, const char *path)
return virCgroupDenyDevice(group, return virCgroupDenyDevice(group,
S_ISCHR(sb.st_mode) ? 'c' : 'b', S_ISCHR(sb.st_mode) ? 'c' : 'b',
major(sb.st_rdev), major(sb.st_rdev),
minor(sb.st_rdev)); minor(sb.st_rdev),
perms);
} }
#else #else
int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED, int virCgroupDenyDevicePath(virCgroupPtr group ATTRIBUTE_UNUSED,
const char *path ATTRIBUTE_UNUSED) const char *path ATTRIBUTE_UNUSED,
int perms ATTRIBUTE_UNUSED)
{ {
return -ENOSYS; return -ENOSYS;
} }

View File

@ -60,27 +60,41 @@ int virCgroupGetMemorySoftLimit(virCgroupPtr group, unsigned long long *kb);
int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb); int virCgroupSetSwapHardLimit(virCgroupPtr group, unsigned long long kb);
int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb); int virCgroupGetSwapHardLimit(virCgroupPtr group, unsigned long long *kb);
enum {
VIR_CGROUP_DEVICE_READ = 1,
VIR_CGROUP_DEVICE_WRITE = 2,
VIR_CGROUP_DEVICE_MKNOD = 4,
VIR_CGROUP_DEVICE_RW = VIR_CGROUP_DEVICE_READ | VIR_CGROUP_DEVICE_WRITE,
VIR_CGROUP_DEVICE_RWM = VIR_CGROUP_DEVICE_RW | VIR_CGROUP_DEVICE_MKNOD,
};
int virCgroupDenyAllDevices(virCgroupPtr group); int virCgroupDenyAllDevices(virCgroupPtr group);
int virCgroupAllowDevice(virCgroupPtr group, int virCgroupAllowDevice(virCgroupPtr group,
char type, char type,
int major, int major,
int minor); int minor,
int perms);
int virCgroupAllowDeviceMajor(virCgroupPtr group, int virCgroupAllowDeviceMajor(virCgroupPtr group,
char type, char type,
int major); int major,
int perms);
int virCgroupAllowDevicePath(virCgroupPtr group, int virCgroupAllowDevicePath(virCgroupPtr group,
const char *path); const char *path,
int perms);
int virCgroupDenyDevice(virCgroupPtr group, int virCgroupDenyDevice(virCgroupPtr group,
char type, char type,
int major, int major,
int minor); int minor,
int perms);
int virCgroupDenyDeviceMajor(virCgroupPtr group, int virCgroupDenyDeviceMajor(virCgroupPtr group,
char type, char type,
int major); int major,
int perms);
int virCgroupDenyDevicePath(virCgroupPtr group, int virCgroupDenyDevicePath(virCgroupPtr group,
const char *path); const char *path,
int perms);
int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares); int virCgroupSetCpuShares(virCgroupPtr group, unsigned long long shares);
int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares); int virCgroupGetCpuShares(virCgroupPtr group, unsigned long long *shares);