6:08 p.m.
Based on some feedback from Steve Grubb, Stephan Mueller, and others
(unfortunately most of it on some non-public lists), I'm proposing the
following patches to enhance my earlier audits for device cgroup ACLs.
Pre-patch, cgroup audits looked like:
type=VIRT_RESOURCE msg=audit(1298068194.479:83142): user pid=23863 uid=0 auid=500 ses=2
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=deny vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=all:
exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=?
terminal=pts/0 res=success'
type=VIRT_RESOURCE msg=audit(1298068194.480:83143): user pid=23863 uid=0 auid=500 ses=2
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=major
type="pty": exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd"
hostname=? addr=? terminal=pts/0 res=success'
type=VIRT_RESOURCE msg=audit(1298068194.480:83145): user pid=23863 uid=0 auid=500 ses=2
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201 item=file
path="/dev/null": exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd"
hostname=? addr=? terminal=pts/0 res=success'
Post-patch, the same three audits are modified to include cgroup
controller, rdev information for files, major device number for
categories, and better names so as not to collide with well-known
audit field names (for example, audit libraries expect item= to match
a decimal integer, so I used class= instead).
type=VIRT_RESOURCE msg=audit(1299541864.111:78295): user pid=30632 uid=0 auid=500 ses=1
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=deny vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201
cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=all:
exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=?
terminal=pts/0 res=success'
type=VIRT_RESOURCE msg=audit(1299541864.112:78296): user pid=30632 uid=0 auid=500 ses=1
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201
cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=major category=pty
maj=88: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=?
terminal=pts/0 res=success'
type=VIRT_RESOURCE msg=audit(1299541864.112:78297): user pid=30632 uid=0 auid=500 ses=1
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='resrc=cgroup
reason=allow vm="fedora_12" uuid=51c6fc83-65a4-e627-b698-042b00145201
cgroup="/cgroup/devices/libvirt/qemu/fedora_12/" class=path path=/dev/null
rdev=01:03: exe="/home/dummy/libvirt/daemon/.libs/lt-libvirtd" hostname=? addr=?
terminal=pts/0 res=success'
Eric Blake (3):
audit: tweak audit messages to match conventions
audit: split cgroup audit types to allow more information
audit: also audit cgroup controller path
src/libvirt_private.syms | 1 +
src/qemu/qemu_audit.c | 115 ++++++++++++++++++++++++++++++++++++++++------
src/qemu/qemu_audit.h | 14 +++++-
src/qemu/qemu_cgroup.c | 29 ++++++------
src/qemu/qemu_driver.c | 8 ++--
src/util/cgroup.c | 8 ++--
src/util/cgroup.h | 5 ++
7 files changed, 142 insertions(+), 38 deletions(-)
--
1.7.4
6:08 p.m.
New subject: [libvirt] [PATCH 1/3] audit: tweak audit messages to match conventions
* src/qemu/qemu_audit.c (qemuDomainHostdevAudit): Avoid use of
"type", which has a pre-defined meaning.
(qemuDomainCgroupAudit): Likewise, as well as "item".
---
src/qemu/qemu_audit.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index b1948c8..0f954c0 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -159,7 +159,7 @@ qemuDomainHostdevAudit(virDomainObjPtr vm,
}
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=dev reason=%s %s uuid=%s type=%s %s",
+ "resrc=dev reason=%s %s uuid=%s bus=%s %s",
reason, vmname, uuidstr,
virDomainHostdevSubsysTypeToString(hostdev->source.subsys.type),
device);
@@ -200,14 +200,14 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm,
return;
}
if (name &&
- !(detail = virAuditEncode(STREQ(item, "path") ? "path" :
"type",
+ !(detail = virAuditEncode(STREQ(item, "path") ? "path" :
"category",
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",
+ "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
reason, vmname, uuidstr,
item, detail ? " " : "", detail ? detail :
"");
--
1.7.4
6:08 p.m.
New subject: [libvirt] [PATCH 2/3] audit: split cgroup audit types to allow more information
Device names can be manipulated, so it is better to also log
the major/minor device number corresponding to the cgroup ACL
changes that libvirt made. This required some refactoring
of the relatively new qemu cgroup audit code.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Drop a parameter.
(qemuDomainCgroupAuditMajor, qemuDomainCgroupAuditPath): New
functions, to allow listing device major/minor in audit.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust callers.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupChardevCgroup, qemuSetupHostUsbDeviceCgroup)
(qemuSetupCgroup, qemuTeardownDiskPathDeny): Likewise.
---
src/qemu/qemu_audit.c | 104 +++++++++++++++++++++++++++++++++++++++++-------
src/qemu/qemu_audit.h | 14 ++++++-
src/qemu/qemu_cgroup.c | 29 +++++++------
src/qemu/qemu_driver.c | 8 ++--
4 files changed, 120 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 0f954c0..0d85fa7 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -23,6 +23,9 @@
#include <config.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
#include "qemu_audit.h"
#include "virtaudit.h"
#include "uuid.h"
@@ -176,9 +179,10 @@ cleanup:
* @vm: domain making the cgroups ACL change
* @cgroup: cgroup that manages the devices
* @reason: either "allow" or "deny"
- * @item: one of "all", "path", or "major"
- * @name: NULL for @item of "all", device path for @item of "path",
and
- * string describing major device type for @item of "major"
+ * @extra: additional details, in the form "all",
+ * "major category=xyz maj=nn", or "path path=xyz dev=nn:mm" (the
+ * latter two are generated by qemuDomainCgroupAuditMajor and
+ * qemuDomainCgroupAuditPath).
* @success: true if the cgroup operation succeeded
*
* Log an audit message about an attempted cgroup device ACL change.
@@ -186,37 +190,107 @@ cleanup:
void qemuDomainCgroupAudit(virDomainObjPtr vm,
virCgroupPtr cgroup ATTRIBUTE_UNUSED,
const char *reason,
- const char *item,
- const char *name,
+ const char *extra,
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, "path") ? "path" :
"category",
- name))) {
+
+ VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+ "resrc=cgroup reason=%s %s uuid=%s class=%s",
+ reason, vmname, uuidstr, extra);
+
+ VIR_FREE(vmname);
+}
+
+/**
+ * qemuDomainCgroupAuditMajor:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @maj: the major number of the device category
+ * @name: a textual name for that device category, alphabetic only
+ * @success: true if the cgroup operation succeeded
+ *
+ * Log an audit message about an attempted cgroup device ACL change.
+ */
+void qemuDomainCgroupAuditMajor(virDomainObjPtr vm,
+ virCgroupPtr cgroup,
+ const char *reason,
+ int maj,
+ const char *name,
+ bool success)
+{
+ char *extra;
+
+ if (virAsprintf(&extra, "major category=%s maj=%02X", name, maj) <
0) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+
+ qemuDomainCgroupAudit(vm, cgroup, reason, extra, success);
+
+ VIR_FREE(extra);
+}
+
+/**
+ * qemuDomainCgroupAuditPath:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @path: the device being adjusted
+ * @success: true if the cgroup operation succeeded
+ *
+ * Log an audit message about an attempted cgroup device ACL change to
+ * a specific device.
+ */
+void qemuDomainCgroupAuditPath(virDomainObjPtr vm,
+ virCgroupPtr cgroup,
+ const char *reason,
+ const char *path,
+ bool success)
+{
+ char *detail;
+ char *extra;
+ int rc;
+
+ if (!(detail = virAuditEncode("path", path))) {
+ VIR_WARN0("OOM while encoding audit message");
+ return;
+ }
+
+#if defined major && defined minor
+ struct stat sb;
+ if (stat(path, &sb) == 0 &&
+ (S_ISCHR(sb.st_mode) || S_ISBLK(sb.st_mode))) {
+ int maj = major(sb.st_rdev);
+ int min = minor(sb.st_rdev);
+ rc = virAsprintf(&extra, "path path=%s rdev=%02X:%02X",
+ path, maj, min);
+ } else
+#endif
+ {
+ rc = virAsprintf(&extra, "path path=%s rdev=?", path);
+ }
+
+ if (rc < 0) {
VIR_WARN0("OOM while encoding audit message");
goto cleanup;
}
- VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
- reason, vmname, uuidstr,
- item, detail ? " " : "", detail ? detail :
"");
+ qemuDomainCgroupAudit(vm, cgroup, reason, extra, success);
cleanup:
- VIR_FREE(vmname);
+ VIR_FREE(extra);
VIR_FREE(detail);
}
-
/**
* qemuDomainResourceAudit:
* @vm: domain making an integer resource change
diff --git a/src/qemu/qemu_audit.h b/src/qemu/qemu_audit.h
index 247edde..500eb98 100644
--- a/src/qemu/qemu_audit.h
+++ b/src/qemu/qemu_audit.h
@@ -46,9 +46,19 @@ void qemuDomainHostdevAudit(virDomainObjPtr vm,
void qemuDomainCgroupAudit(virDomainObjPtr vm,
virCgroupPtr group,
const char *reason,
- const char *item,
- const char *name,
+ const char *extra,
bool success);
+void qemuDomainCgroupAuditMajor(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ int maj,
+ const char *name,
+ bool success);
+void qemuDomainCgroupAuditPath(virDomainObjPtr vm,
+ virCgroupPtr group,
+ const char *reason,
+ const char *path,
+ bool success);
void qemuDomainMemoryAudit(virDomainObjPtr vm,
unsigned long long oldmem,
unsigned long long newmem,
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index e71d3fa..b911005 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -68,8 +68,8 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
/* XXX RO vs RW */
rc = virCgroupAllowDevicePath(data->cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path", path,
- rc == 0);
+ qemuDomainCgroupAuditPath(data->vm, data->cgroup, "allow", path,
+ rc == 0);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -111,8 +111,8 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
/* XXX RO vs RW */
rc = virCgroupDenyDevicePath(data->cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "deny",
"path", path,
- rc == 0);
+ qemuDomainCgroupAuditPath(data->vm, data->cgroup, "deny", path,
+ rc == 0);
if (rc < 0) {
if (rc == -EACCES) { /* Get this for root squash NFS */
VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -156,8 +156,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);
if (rc < 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path",
- dev->source.data.file.path, rc == 0);
+ qemuDomainCgroupAuditPath(data->vm, data->cgroup, "allow",
+ dev->source.data.file.path, rc == 0);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -179,8 +179,8 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
VIR_DEBUG("Process path '%s' for USB device", path);
rc = virCgroupAllowDevicePath(data->cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(data->vm, data->cgroup, "allow",
"path", path,
- rc == 0);
+ qemuDomainCgroupAuditPath(data->vm, data->cgroup, "allow", path,
+ rc == 0);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s"),
@@ -216,7 +216,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);
+ qemuDomainCgroupAudit(vm, cgroup, "deny", "all", rc == 0);
if (rc != 0) {
if (rc == -EPERM) {
VIR_WARN0("Group devices ACL is not accessible, disabling
whitelisting");
@@ -234,7 +234,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
}
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"pty", rc == 0);
+ qemuDomainCgroupAuditMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
+ "pty", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/pts/ devices"));
@@ -247,8 +248,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
driver->vncAllowHostAudio) ||
(vm->def->graphics[0]->type == VIR_DOMAIN_GRAPHICS_TYPE_SDL)))) {
rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_SND_MAJOR);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "major",
"sound",
- rc == 0);
+ qemuDomainCgroupAuditMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
+ "sound", rc == 0);
if (rc != 0) {
virReportSystemError(-rc, "%s",
_("unable to allow /dev/snd/ devices"));
@@ -259,8 +260,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
for (i = 0; deviceACL[i] != NULL ; i++) {
rc = virCgroupAllowDevicePath(cgroup,
deviceACL[i]);
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path",
- deviceACL[i], rc == 0);
+ qemuDomainCgroupAuditPath(vm, cgroup, "allow",
+ 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 0f7cbad..cb5d7ef 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1965,7 +1965,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
}
rc = virCgroupAllowDevicePath(cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "allow", "path", path,
rc == 0);
+ qemuDomainCgroupAuditPath(vm, cgroup, "allow", path, rc == 0);
if (rc < 0) {
virReportSystemError(-rc,
_("Unable to allow device %s for %s"),
@@ -2016,7 +2016,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver,
virDomainPtr dom,
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path,
rc == 0);
+ qemuDomainCgroupAuditPath(vm, cgroup, "deny", path, rc == 0);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s %d",
path, vm->def->name, rc);
@@ -2049,8 +2049,8 @@ endjob:
if (cgroup != NULL) {
rc = virCgroupDenyDevicePath(cgroup, path);
if (rc <= 0)
- qemuDomainCgroupAudit(vm, cgroup, "deny", "path",
path,
- rc == 0);
+ qemuDomainCgroupAuditPath(vm, cgroup, "deny", path,
+ rc == 0);
if (rc < 0)
VIR_WARN("Unable to deny device %s for %s: %d",
path, vm->def->name, rc);
--
1.7.4
6:08 p.m.
New subject: [libvirt] [PATCH 3/3] audit: also audit cgroup controller path
Although the cgroup device ACL controller path can be worked out
by researching the code, it is more efficient to include that
information directly in the audit message.
* src/util/cgroup.h (virCgroupPathOfController): New prototype.
* src/util/cgroup.c (virCgroupPathOfController): Export.
* src/libvirt_private.syms: Likewise.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Use it.
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_audit.c | 19 ++++++++++++++++---
src/util/cgroup.c | 8 ++++----
src/util/cgroup.h | 5 +++++
4 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index efcf3c5..c0da78e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -79,6 +79,7 @@ virCgroupKill;
virCgroupKillRecursive;
virCgroupKillPainfully;
virCgroupMounted;
+virCgroupPathOfController;
virCgroupRemove;
virCgroupSetBlkioWeight;
virCgroupSetCpuShares;
diff --git a/src/qemu/qemu_audit.c b/src/qemu/qemu_audit.c
index 0d85fa7..c2402b5 100644
--- a/src/qemu/qemu_audit.c
+++ b/src/qemu/qemu_audit.c
@@ -188,13 +188,15 @@ cleanup:
* Log an audit message about an attempted cgroup device ACL change.
*/
void qemuDomainCgroupAudit(virDomainObjPtr vm,
- virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+ virCgroupPtr cgroup,
const char *reason,
const char *extra,
bool success)
{
char uuidstr[VIR_UUID_STRING_BUFLEN];
char *vmname;
+ char *controller = NULL;
+ char *detail;
virUUIDFormat(vm->def->uuid, uuidstr);
if (!(vmname = virAuditEncode("vm", vm->def->name))) {
@@ -202,11 +204,22 @@ void qemuDomainCgroupAudit(virDomainObjPtr vm,
return;
}
+ virCgroupPathOfController(cgroup, VIR_CGROUP_CONTROLLER_DEVICES,
+ NULL, &controller);
+
+ if (!(detail = virAuditEncode("cgroup", VIR_AUDIT_STR(controller)))) {
+ VIR_WARN0("OOM while encoding audit message");
+ goto cleanup;
+ }
+
VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
- "resrc=cgroup reason=%s %s uuid=%s class=%s",
- reason, vmname, uuidstr, extra);
+ "resrc=cgroup reason=%s %s uuid=%s %s class=%s",
+ reason, vmname, uuidstr, detail, extra);
+cleanup:
VIR_FREE(vmname);
+ VIR_FREE(controller);
+ VIR_FREE(detail);
}
/**
diff --git a/src/util/cgroup.c b/src/util/cgroup.c
index c5b8cdd..a16b1ab 100644
--- a/src/util/cgroup.c
+++ b/src/util/cgroup.c
@@ -254,10 +254,10 @@ static int virCgroupDetect(virCgroupPtr group)
#endif
-static int virCgroupPathOfController(virCgroupPtr group,
- int controller,
- const char *key,
- char **path)
+int virCgroupPathOfController(virCgroupPtr group,
+ int controller,
+ const char *key,
+ char **path)
{
if (controller == -1) {
int i;
diff --git a/src/util/cgroup.h b/src/util/cgroup.h
index d468cb3..b3c5f27 100644
--- a/src/util/cgroup.h
+++ b/src/util/cgroup.h
@@ -40,6 +40,11 @@ int virCgroupForDomain(virCgroupPtr driver,
virCgroupPtr *group,
int create);
+int virCgroupPathOfController(virCgroupPtr group,
+ int controller,
+ const char *key,
+ char **path);
+
int virCgroupAddTask(virCgroupPtr group, pid_t pid);
int virCgroupSetBlkioWeight(virCgroupPtr group, unsigned int weight);
--
1.7.4
5009
days inactive
5009
days old
3 comments
1 participants
participants (1)
-
Eric Blake