8:59 p.m.
I'm following up on danpb's patches to add initial audit support to
qemu actions (see around commit 8dc136b in Oct 2010). Another useful
thing to audit is all changes to the device ACL whitelist via the
cgroup device controller - namely, any time that the qemu cgroup is
altered to allow or deny access to a (set of) devices.
I saw an alternate suggestion for collecting the needed audit
information by using an inotify script outside of libvirt that
monitors changes to the cgroup file system corresponding to each qemu
instance; however, this had the drawback that it can only monitor that
a change is being attempted, but not the actual change being made. It
is necessary to use libvirt to provide the audit information, in order
to have a rich enough set of information to make the audit worthwhile.
I believe this patch series catches all instances where libvirt
modifies the whitelist for a qemu instance, but I didn't do anything
for when libvirt first creates a separate cgroup device whitelist as
part of starting a new guest. Also, I'm not sure if the amount of
information passed to the audit is adequate, or if it needs tweaking.
Also, libvirt blindly passes a long list of paths through the
low-level util/cgroup.c functions, including non-devices where it
ignores the EINVAL failure later. But this could pollute the audit
log with entries corresponding to cases where altering the actual
cgroup was never even attempted. Should I tweak things to only do an
audit when an actual cgroup change was attempted (perhaps by modifying
util/cgroup.c to return 0 on success, 1 on skip, and -errno on
failure, rather than the current -EINVAL on skip)?
This is post-0.8.8 material.
Eric Blake (2):
audit: prepare qemu for listing vm in cgroup audits
audit: add qemu hooks for auditing cgroup events
src/qemu/qemu_audit.c | 48 ++++++++++++++++++++++++++++++-
src/qemu/qemu_audit.h | 9 +++++-
src/qemu/qemu_cgroup.c | 73 +++++++++++++++++++++++++++++++----------------
src/qemu/qemu_cgroup.h | 21 ++++++--------
src/qemu/qemu_driver.c | 12 +++++--
src/qemu/qemu_hotplug.c | 7 ++--
6 files changed, 124 insertions(+), 46 deletions(-)
--
1.7.4
9 p.m.
New subject: [libvirt] [RFC PATCH 1/2] audit: prepare qemu for listing vm in cgroup audits
* src/qemu/qemu_cgroup.h (struct qemuCgroupData): New helper type.
(qemuSetupDiskPathAllow, qemuSetupChardevCgroup)
(qemuTeardownDiskPathDeny): Drop unneeded prototypes.
(qemuSetupDiskCgroup, qemuTeardownDiskCgroup): Adjust prototype.
* src/qemu/qemu_cgroup.c
(qemuSetupDiskPathAllow, qemuSetupChardevCgroup)
(qemuTeardownDiskPathDeny): Mark static and use new type.
(qemuSetupHostUsbDeviceCgroup): Use new type.
(qemuSetupDiskCgroup): Alter signature.
(qemuSetupCgroup): Adjust caller.
* src/qemu/qemu_hotplug.c (qemuDomainAttachHostUsbDevice)
(qemuDomainDetachPciDiskDevice, qemuDomainDetachSCSIDiskDevice):
Likewise.
* src/qemu/qemu_driver.c (qemudDomainAttachDevice)
(qemuDomainUpdateDeviceFlags): Likewise.
---
Relatively straight-forward - all existing qemu audit entries include
the vm being altered; so we need to pass vm through to the
lowest-level points in qemu that alter the cgroup device whitelist.
src/qemu/qemu_cgroup.c | 58 ++++++++++++++++++++++++++--------------------
src/qemu/qemu_cgroup.h | 21 +++++++---------
src/qemu/qemu_driver.c | 8 +++---
src/qemu/qemu_hotplug.c | 7 +++--
4 files changed, 50 insertions(+), 44 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 8cd6ce9..38eacfb 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -54,18 +54,18 @@ int qemuCgroupControllerActive(struct qemud_driver *driver,
return 0;
}
-
-int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
- const char *path,
- size_t depth ATTRIBUTE_UNUSED,
- void *opaque)
+static int
+qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+ const char *path,
+ size_t depth ATTRIBUTE_UNUSED,
+ void *opaque)
{
- virCgroupPtr cgroup = opaque;
+ qemuCgroupData *data = opaque;
int rc;
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
- rc = virCgroupAllowDevicePath(cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path);
if (rc != 0) {
/* Get this for non-block devices */
if (rc == -EINVAL) {
@@ -84,28 +84,31 @@ int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
int qemuSetupDiskCgroup(struct qemud_driver *driver,
+ virDomainObjPtr vm,
virCgroupPtr cgroup,
virDomainDiskDefPtr disk)
{
+ qemuCgroupData data = { vm, cgroup };
return virDomainDiskDefForeachPath(disk,
driver->allowDiskFormatProbing,
true,
qemuSetupDiskPathAllow,
- cgroup);
+ &data);
}
-int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
- const char *path,
- size_t depth ATTRIBUTE_UNUSED,
- void *opaque)
+static int
+qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+ const char *path,
+ size_t depth ATTRIBUTE_UNUSED,
+ void *opaque)
{
- virCgroupPtr cgroup = opaque;
+ qemuCgroupData *data = opaque;
int rc;
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
- rc = virCgroupDenyDevicePath(cgroup, path);
+ rc = virCgroupDenyDevicePath(data->cgroup, path);
if (rc != 0) {
/* Get this for non-block devices */
if (rc == -EINVAL) {
@@ -124,22 +127,25 @@ int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk
ATTRIBUTE_UNUSED,
int qemuTeardownDiskCgroup(struct qemud_driver *driver,
+ virDomainObjPtr vm,
virCgroupPtr cgroup,
virDomainDiskDefPtr disk)
{
+ qemuCgroupData data = { vm, cgroup };
return virDomainDiskDefForeachPath(disk,
driver->allowDiskFormatProbing,
true,
qemuTeardownDiskPathDeny,
- cgroup);
+ &data);
}
-int qemuSetupChardevCgroup(virDomainDefPtr def,
- virDomainChrDefPtr dev,
- void *opaque)
+static int
+qemuSetupChardevCgroup(virDomainDefPtr def,
+ virDomainChrDefPtr dev,
+ void *opaque)
{
- virCgroupPtr cgroup = opaque;
+ qemuCgroupData *data = opaque;
int rc;
if (dev->source.type != VIR_DOMAIN_CHR_TYPE_DEV)
@@ -147,7 +153,7 @@ int qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk",
dev->source.data.file.path);
- rc = virCgroupAllowDevicePath(cgroup, dev->source.data.file.path);
+ rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -163,11 +169,11 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
const char *path,
void *opaque)
{
- virCgroupPtr cgroup = opaque;
+ qemuCgroupData *data = opaque;
int rc;
VIR_DEBUG("Process path '%s' for USB device", path);
- rc = virCgroupAllowDevicePath(cgroup, path);
+ rc = virCgroupAllowDevicePath(data->cgroup, path);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -201,6 +207,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
+ qemuCgroupData data = { vm, cgroup };
rc = virCgroupDenyAllDevices(cgroup);
if (rc != 0) {
if (rc == -EPERM) {
@@ -214,7 +221,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
for (i = 0; i < vm->def->ndisks ; i++) {
- if (qemuSetupDiskCgroup(driver, cgroup, vm->def->disks[i]) < 0)
+ if (qemuSetupDiskCgroup(driver, vm, cgroup, vm->def->disks[i]) < 0)
goto cleanup;
}
@@ -249,7 +256,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (virDomainChrDefForeach(vm->def,
true,
qemuSetupChardevCgroup,
- cgroup) < 0)
+ &data) < 0)
goto cleanup;
for (i = 0; i < vm->def->nhostdevs; i++) {
@@ -265,7 +272,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
hostdev->source.subsys.u.usb.device)) == NULL)
goto cleanup;
- if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, cgroup) < 0 )
+ if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup,
+ &data) < 0)
goto cleanup;
}
}
diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h
index 0a9692b..299bd2d 100644
--- a/src/qemu/qemu_cgroup.h
+++ b/src/qemu/qemu_cgroup.h
@@ -1,7 +1,7 @@
/*
* qemu_cgroup.h: QEMU cgroup management
*
- * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -28,25 +28,22 @@
# include "domain_conf.h"
# include "qemu_conf.h"
+struct _qemuCgroupData {
+ virDomainObjPtr vm;
+ virCgroupPtr cgroup;
+};
+typedef struct _qemuCgroupData qemuCgroupData;
+
int qemuCgroupControllerActive(struct qemud_driver *driver,
int controller);
-int qemuSetupDiskPathAllow(virDomainDiskDefPtr disk,
- const char *path,
- size_t depth,
- void *opaque);
int qemuSetupDiskCgroup(struct qemud_driver *driver,
+ virDomainObjPtr vm,
virCgroupPtr cgroup,
virDomainDiskDefPtr disk);
-int qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk,
- const char *path,
- size_t depth,
- void *opaque);
int qemuTeardownDiskCgroup(struct qemud_driver *driver,
+ virDomainObjPtr vm,
virCgroupPtr cgroup,
virDomainDiskDefPtr disk);
-int qemuSetupChardevCgroup(virDomainDefPtr def,
- virDomainChrDefPtr dev,
- void *opaque);
int qemuSetupHostUsbDeviceCgroup(usbDevice *dev,
const char *path,
void *opaque);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a2210..375ad2b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6559,7 +6559,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
vm->def->name);
goto endjob;
}
- if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
goto endjob;
}
@@ -6605,7 +6605,7 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
/* Fallthrough */
}
if (ret != 0 && cgroup) {
- if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
VIR_WARN("Failed to teardown cgroup for disk path %s",
NULLSTR(dev->data.disk->src));
}
@@ -6730,7 +6730,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
vm->def->name);
goto endjob;
}
- if (qemuSetupDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuSetupDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
goto endjob;
}
@@ -6754,7 +6754,7 @@ static int qemuDomainUpdateDeviceFlags(virDomainPtr dom,
}
if (ret != 0 && cgroup) {
- if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
VIR_WARN("Failed to teardown cgroup for disk path %s",
NULLSTR(dev->data.disk->src));
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index fb9db5a..e959151 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -893,6 +893,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver,
if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
virCgroupPtr cgroup = NULL;
usbDevice *usb;
+ qemuCgroupData data = { vm, cgroup };
if (virCgroupForDomain(driver->cgroup, vm->def->name, &cgroup, 0)
!=0 ) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
@@ -905,7 +906,7 @@ int qemuDomainAttachHostUsbDevice(struct qemud_driver *driver,
hostdev->source.subsys.u.usb.device)) == NULL)
goto error;
- if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, cgroup) < 0 )
+ if (usbDeviceFileIterate(usb, qemuSetupHostUsbDeviceCgroup, &data) < 0)
goto error;
}
@@ -1206,7 +1207,7 @@ int qemuDomainDetachPciDiskDevice(struct qemud_driver *driver,
VIR_WARN("Unable to restore security label on %s",
dev->data.disk->src);
if (cgroup != NULL) {
- if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
VIR_WARN("Failed to teardown cgroup for disk path %s",
NULLSTR(dev->data.disk->src));
}
@@ -1284,7 +1285,7 @@ int qemuDomainDetachSCSIDiskDevice(struct qemud_driver *driver,
VIR_WARN("Unable to restore security label on %s",
dev->data.disk->src);
if (cgroup != NULL) {
- if (qemuTeardownDiskCgroup(driver, cgroup, dev->data.disk) < 0)
+ if (qemuTeardownDiskCgroup(driver, vm, cgroup, dev->data.disk) < 0)
VIR_WARN("Failed to teardown cgroup for disk path %s",
NULLSTR(dev->data.disk->src));
}
--
1.7.4
9 p.m.
New subject: [libvirt] [RFC PATCH 2/2] audit: add qemu hooks for auditing cgroup events
* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): New prototype.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Implement it.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Add audit.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup)
(qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise.
---
Adding the actual audits. So far, this compiles, but I have no idea
how to test it. Hence posting this as an RFC. Advice of where to
proceed from here would be welcome!
src/qemu/qemu_audit.c | 48 +++++++++++++++++++++++++++++++++++++++++++++++-
src/qemu/qemu_audit.h | 9 ++++++++-
src/qemu/qemu_cgroup.c | 15 +++++++++++++++
src/qemu/qemu_driver.c | 4 ++++
4 files changed, 74 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index f03f984..c76d49e 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -1,7 +1,7 @@
/*
* qemu_audit.c: QEMU audit management
*
- * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -102,6 +102,52 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
}
+/**
+ * qemuDomainCgroupAudit:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @item: one of "all", "file", or "major"
+ * @name: NULL for @item of "all", path for @item of "file", and
+ * string describing major device type for @item of "major"
+ * @success: true if the cgroup operation succeeded
+ *
+ * Log an audit message about an attempted cgroup device ACL change.
+ */
+void qemuDomainCgroupAudit(virDomainObjPtr vm,
+ virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+ const char *reason,
+ const char *item,
+ const char *name,
+ bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char *vmname;
+ char *detail = NULL;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+ if (!(vmname = virAuditEncode("vm", vm->def->name))) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+ if (name &&
+ !(detail = virAuditEncode(STREQ(item, "file") ? "path" :
"type",
+ name))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=cgroup reason=%s %s uuid=%s item=%s%s%s",
+ reason, vmname, uuidstr,
+ item, detail ? " " : "", detail ? detail :
"");
+
+cleanup:
+ VIR_FREE(vmname);
+ VIR_FREE(detail);
+}
+
+
static void qemuDomainLifecycleAudit(virDomainObjPtr vm,
const char *op,
const char *reason,
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 5b5a5d3..40f4591 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -1,7 +1,7 @@
/*
* qemu_audit.h: QEMU audit management
*
- * Copyright (C) 2006-2007, 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -25,6 +25,7 @@
# define __QEMU_AUDIT_H__
# include "domain_conf.h"
+# include "cgroup.h"
void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool success);
void qemuDomainStopAudit(virDomainObjPtr vm, const char *reason);
@@ -38,6 +39,12 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
virDomainNetDefPtr newDef,
const char *reason,
bool success);
+void qemuDomainCgroupAudit(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *item,
+ const char *name,
+ bool success);
void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success);
#endif /* __QEMU_AUDIT_H__ */
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 38eacfb..54665fe 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -29,6 +29,7 @@
#include "memory.h"
#include "virterror_internal.h"
#include "util.h"
+#include "qemu_audit.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -66,6 +67,8 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
rc = virCgroupAllowDevicePath(data->cgroup, path);
+ qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"file", path,
+ rc == 0);
if (rc != 0) {
/* Get this for non-block devices */
if (rc == -EINVAL) {
@@ -109,6 +112,8 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path %s for disk", path);
/* XXX RO vs RW */
rc = virCgroupDenyDevicePath(data->cgroup, path);
+ qemuDomainCgroupAudit(data->vm, data->cgroup, "deny",
"file", path,
+ rc == 0);
if (rc != 0) {
/* Get this for non-block devices */
if (rc == -EINVAL) {
@@ -154,6 +159,8 @@ qemuSetupChardevCgroup(virDomainDefPtr def,
VIR_DEBUG("Process path '%s' for disk",
dev->source.data.file.path);
rc = virCgroupAllowDevicePath(data->cgroup, dev->source.data.file.path);
+ qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"file",
+ dev->source.data.file.path, rc == 0);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -174,6 +181,8 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path '%s' for USB device", path);
rc = virCgroupAllowDevicePath(data->cgroup, path);
+ qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"file", path,
+ rc == 0);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -209,6 +218,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (qemuCgroupControllerActive(driver, VIR_CGROUP_CONTROLLER_DEVICES)) {
qemuCgroupData data = { vm, cgroup };
rc = virCgroupDenyAllDevices(cgroup);
+ qemuDomainCgroupAudit(vm, cgroup, "deny", "all", NULL, rc ==
0);
if (rc != 0) {
if (rc == -EPERM) {
VIR_WARN0("Group devices ACL is not accessible, disabling
whitelisting");
@@ -226,6 +236,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
+ qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"pty", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/pts/ devices"));
@@ -234,6 +245,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
if (vm->def->nsounds) {
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
+ qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"sound",
+ rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/snd/ devices"));
@@ -244,6 +257,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup,
deviceACL[i]);
+ qemuDomainCgroupAudit(vm, cgroup, "allow", "file",
+ deviceACL[i], rc == 0);
if (rc < 0 &&
rc != -ENOENT) {
virReportSystemError(-rc,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 375ad2b..d89187a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4499,6 +4499,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
goto endjob;
}
rc = virCgroupAllowDevicePath(cgroup, path);
+ qemuDomainCgroupAudit(vm, cgroup, "allow", "file", path, rc
== 0);
if (rc != 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -4548,6 +4549,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
+ qemuDomainCgroupAudit(vm, cgroup, "deny", "file", path, rc ==
0);
if (rc != 0)
VIR_WARN("Unable to deny device %s for %s %d",
path, vm->def->name, rc);
@@ -4579,6 +4581,8 @@ endjob:
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
+ qemuDomainCgroupAudit(vm, cgroup, "deny", "file",
path,
+ rc == 0);
if (rc != 0)
VIR_WARN("Unable to deny device %s for %s: %d",
path, vm->def->name, rc);
--
1.7.4
2:15 p.m.
On Tuesday, February 15, 2011 09:59:59 pm Eric Blake wrote:
I'm following up on danpb's patches to add initial audit
support to
qemu actions (see around commit 8dc136b in Oct 2010). Another useful
thing to audit is all changes to the device ACL whitelist via the
cgroup device controller - namely, any time that the qemu cgroup is
altered to allow or deny access to a (set of) devices.
I saw an alternate suggestion for collecting the needed audit
information by using an inotify script outside of libvirt that
monitors changes to the cgroup file system corresponding to each qemu
instance; however, this had the drawback that it can only monitor that
a change is being attempted, but not the actual change being made. It
is necessary to use libvirt to provide the audit information, in order
to have a rich enough set of information to make the audit worthwhile.
Does the cgroup auditing belong in the kernel? Or is there a syscall that would catch
this?
I believe this patch series catches all instances where libvirt
modifies the whitelist for a qemu instance, but I didn't do anything
for when libvirt first creates a separate cgroup device whitelist as
part of starting a new guest. Also, I'm not sure if the amount of
information passed to the audit is adequate, or if it needs tweaking.
We would need to see some sample events. This patch should also be reviewed on the
rhel6-cc-external-list so that Stephan can weigh in on whether you have everything
he's expecting.
Also, libvirt blindly passes a long list of paths through the
low-level util/cgroup.c functions, including non-devices where it
ignores the EINVAL failure later. But this could pollute the audit
log with entries corresponding to cases where altering the actual
cgroup was never even attempted.
The audit event log needs to be accurate. You should never put events in there that
aren't true.
Should I tweak things to only do an audit when an actual cgroup
change was attempted
(perhaps by modifying util/cgroup.c to return 0 on success, 1 on skip, and -errno on
failure, rather than the current -EINVAL on skip)?
Probably. But also be careful about skipping too many things because there may be
things with -EPERM/EACCES that we might want. I think I'd discuss this on the rhel6-cc
list for clarification.
-Steve
6:34 p.m.
New subject: [libvirt] [RFC PATCH 3/2] audit: audit qemu memory and vcpu adjusments
* src/qemu/qemu_audit.h (qemuDomainMemoryAudit)
(qemuDomainVcpuAudit): New prototypes.
* src/qemu/qemu_audit.c (qemuDomainResourceAudit)
(qemuDomainMemoryAudit, qemuDomainVcpuAudit): New functions.
(qemuDomainStartAudit): Call as appropriate.
* src/qemu/qemu_driver.c (qemudDomainSetMemory)
(qemudDomainHotplugVcpus): Likewise.
---
Another round of useful audit points, for tracking hotplug
changes to vcpus and memory allocations. Again, I'm not sure
if this is the correct amount of information, hence including
this as a continuation of my RFC patches.
I'm not sure if qemudDomainSetMemory is doing the right thing -
it doesn't update vm->def->mem.cur_balloon on success; on the
other hand, dumpXML always recalculates the cur_balloon from a
monitor query, and I don't know if any other API exposes a stale
cur_balloon value from the vm definition.
src/qemu/qemu_audit.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_audit.h | 10 ++++++++
src/qemu/qemu_driver.c | 7 +++++-
3 files changed, 72 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index c76d49e..6ea31c9 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -148,6 +148,59 @@ cleanup:
}
+/**
+ * qemuDomainResourceAudit:
+ * @vm: domain making an integer resource change
+ * @resource: name of the resource: "mem" or "vcpu"
+ * @oldval: the old value of the resource
+ * @newval: the new value of the resource
+ * @reason: either "start" or "update"
+ * @success: true if the resource change succeeded
+ *
+ * Log an audit message about an attempted resource change.
+ */
+static void
+qemuDomainResourceAudit(virDomainObjPtr vm,
+ const char *resource,
+ unsigned long long oldval,
+ unsigned long long newval,
+ const char *reason,
+ bool success)
+{
+ char uuidstr[VIR_UUID_STRING_BUFLEN];
+ char *vmname;
+
+ virUUIDFormat(vm->def->uuid, uuidstr);
+ if (!(vmname = virAuditEncode("vm", vm->def->name))) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=%s reason=%s %s uuid=%s old-%s=%lld new-%s=%lld",
+ resource, reason, vmname, uuidstr,
+ resource, oldval, resource, newval);
+
+ VIR_FREE(vmname);
+}
+
+void
+qemuDomainMemoryAudit(virDomainObjPtr vm,
+ unsigned long long oldmem, unsigned long long newmem,
+ const char *reason, bool success)
+{
+ return qemuDomainResourceAudit(vm, "mem", oldmem, newmem, reason,
success);
+}
+
+void
+qemuDomainVcpuAudit(virDomainObjPtr vm,
+ unsigned int oldvcpu, unsigned int newvcpu,
+ const char *reason, bool success)
+{
+ return qemuDomainResourceAudit(vm, "vcpu", oldvcpu, newvcpu, reason,
+ success);
+}
+
static void qemuDomainLifecycleAudit(virDomainObjPtr vm,
const char *op,
const char *reason,
@@ -185,6 +238,9 @@ void qemuDomainStartAudit(virDomainObjPtr vm, const char *reason, bool
success)
qemuDomainNetAudit(vm, NULL, net, "start", true);
}
+ qemuDomainMemoryAudit(vm, 0, vm->def->mem.cur_balloon, "start",
true);
+ qemuDomainVcpuAudit(vm, 0, vm->def->vcpus, "start", true);
+
qemuDomainLifecycleAudit(vm, "start", reason, success);
}
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 40f4591..cdbb957 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -45,6 +45,16 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm,
const char *item,
const char *name,
bool success);
+void qemuDomainMemoryAudit(virDomainObjPtr vm,
+ unsigned long long oldmem,
+ unsigned long long newmem,
+ const char *reason,
+ bool success);
+void qemuDomainVcpuAudit(virDomainObjPtr vm,
+ unsigned int oldvcpu,
+ unsigned int newvcpu,
+ const char *reason,
+ bool success);
void qemuDomainSecurityLabelAudit(virDomainObjPtr vm, bool success);
#endif /* __QEMU_AUDIT_H__ */
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index a98ceb4..ddd7728 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1605,6 +1605,8 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long
newmem) {
qemuDomainObjEnterMonitor(vm);
r = qemuMonitorSetBalloon(priv->mon, newmem);
qemuDomainObjExitMonitor(vm);
+ qemuDomainMemoryAudit(vm, vm->def->mem.cur_balloon, newmem,
"update",
+ r <= 0);
if (r < 0)
goto endjob;
@@ -1615,6 +1617,7 @@ static int qemudDomainSetMemory(virDomainPtr dom, unsigned long
newmem) {
goto endjob;
}
+ /* XXX update vm->def->mem.cur_balloon? */
ret = 0;
endjob:
if (qemuDomainObjEndJob(vm) == 0)
@@ -2518,8 +2521,9 @@ static void processWatchdogEvent(void *data, void *opaque)
static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
- int i, rc;
+ int i, rc = 1;
int ret = -1;
+ int oldvcpus = vm->def->vcpus;
qemuDomainObjEnterMonitor(vm);
@@ -2554,6 +2558,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int
nvcpus)
cleanup:
qemuDomainObjExitMonitor(vm);
+ qemuDomainVcpuAudit(vm, oldvcpus, nvcpus, "update", rc <= 0);
return ret;
unsupported:
--
1.7.4
5022
days inactive
5028
days old
4 comments
2 participants
participants (2)
-
Eric Blake -
Steve Grubb