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.

Also, qemuSetupChardevCgroup was only auditing on failure, not success.

* src/qemu/qemu_audit.h (qemuDomainCgroupAudit): Delete.
(qemuAuditCgroup, qemuAuditCgroupMajor, qemuAuditCgroupPath): New
prototypes.
* src/qemu/qemu_audit.c (qemuDomainCgroupAudit): Rename...
(qemuAuditCgroup): ...and drop a parameter.
(qemuAuditCgroupMajor, qemuAuditCgroupPath): New functions, to
allow listing device major/minor in audit.
(qemuAuditGetRdev): New helper function.
* src/qemu/qemu_driver.c (qemudDomainSaveFlag): Adjust callers.
* src/qemu/qemu_cgroup.c (qemuSetupDiskPathAllow)
(qemuSetupHostUsbDeviceCgroup, qemuSetupCgroup)
(qemuTeardownDiskPathDeny): Likewise.
(qemuSetupChardevCgroup): Likewise, fixing missing audit.

src/qemu/qemu_audit.c

@@ -23,12 +23,40 @@
 
 #include <config.h>
 
+#include <sys/stat.h>
+#include <sys/types.h>
+
 #include "qemu_audit.h"
 #include "virtaudit.h"
 #include "uuid.h"
 #include "logging.h"
 #include "memory.h"
 
+/* Return nn:mm in hex for block and character devices, and NULL
+ * for other file types, stat failure, or allocation failure.  */
+#if defined major && defined minor
+static char *
+qemuAuditGetRdev(const char *path)
+{
+    char *ret = NULL;
+    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);
+        virAsprintf(&ret, "%02X:%02X", maj, min);
+    }
+    return ret;
+}
+#else
+static char *
+qemuAuditGetRdev(const char *path ATTRIBUTE_UNUSED)
+{
+    return NULL;
+}
+#endif
+
 void qemuDomainDiskAudit(virDomainObjPtr vm,
                          virDomainDiskDefPtr oldDef,
                          virDomainDiskDefPtr newDef,
@@ -106,7 +134,7 @@ void qemuDomainNetAudit(virDomainObjPtr vm,
  * qemuDomainHostdevAudit:
  * @vm: domain making a change in pass-through host device
  * @hostdev: device being attached or removed
- * @reason: one of "start, "attach", or "detach"
+ * @reason: one of "start", "attach", or "detach"
  * @success: true if the device passthrough operation succeeded
  *
  * Log an audit message about an attempted device passthrough change.
@@ -172,51 +200,106 @@ cleanup:
 
 
 /**
- * qemuDomainCgroupAudit:
+ * qemuAuditCgroup:
  * @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"
+ * @extra: additional details, in the form "all",
- * @name: NULL for @item of "all", device path for @item of "path", and
+ * "major category=xyz maj=nn", or "path path=xyz dev=nn:mm" (the
- * string describing major device type for @item of "major"
+ * latter two are generated by qemuAuditCgroupMajor and
+ * qemuAuditCgroupPath).
  * @success: true if the cgroup operation succeeded
  *
  * Log an audit message about an attempted cgroup device ACL change.
  */
-void qemuDomainCgroupAudit(virDomainObjPtr vm,
+void
-                           virCgroupPtr cgroup ATTRIBUTE_UNUSED,
+qemuAuditCgroup(virDomainObjPtr vm, virCgroupPtr cgroup ATTRIBUTE_UNUSED,
-                           const char *reason,
+                const char *reason, const char *extra, bool success)
-                           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, "path") ? "path" : "category",
+    VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
-                                  name))) {
+              "resrc=cgroup reason=%s %s uuid=%s class=%s",
+              reason, vmname, uuidstr, extra);
+
+    VIR_FREE(vmname);
+}
+
+/**
+ * qemuAuditCgroupMajor:
+ * @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
+qemuAuditCgroupMajor(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;
+    }
+
+    qemuAuditCgroup(vm, cgroup, reason, extra, success);
+
+    VIR_FREE(extra);
+}
+
+/**
+ * qemuAuditCgroupPath:
+ * @vm: domain making the cgroups ACL change
+ * @cgroup: cgroup that manages the devices
+ * @reason: either "allow" or "deny"
+ * @path: the device being adjusted
+ * @rc: > 0 if not a device, 0 if success, < 0 if failure
+ *
+ * Log an audit message about an attempted cgroup device ACL change to
+ * a specific device.
+ */
+void
+qemuAuditCgroupPath(virDomainObjPtr vm, virCgroupPtr cgroup,
+                    const char *reason, const char *path, int rc)
+{
+    char *detail;
+    char *rdev;
+    char *extra;
+
+    /* Nothing to audit for regular files.  */
+    if (rc > 0)
+        return;
+
+    rdev = qemuAuditGetRdev(path);
+
+    if (!(detail = virAuditEncode("path", path)) ||
+        virAsprintf(&extra, "path path=%s rdev=%s",
+                    path, VIR_AUDIT_STR(rdev)) < 0) {
         VIR_WARN0("OOM while encoding audit message");
         goto cleanup;
     }
 
-    VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
+    qemuAuditCgroup(vm, cgroup, reason, extra, rc == 0);
-              "resrc=cgroup reason=%s %s uuid=%s class=%s%s%s",
-              reason, vmname, uuidstr,
-              item, detail ? " " : "", detail ? detail : "");
 
 cleanup:
-    VIR_FREE(vmname);
+    VIR_FREE(extra);
     VIR_FREE(detail);
+    VIR_FREE(rdev);
 }
 
-
 /**
  * qemuDomainResourceAudit:
  * @vm: domain making an integer resource change

src/qemu/qemu_audit.h

@@ -43,12 +43,22 @@ void qemuDomainHostdevAudit(virDomainObjPtr vm,
                             virDomainHostdevDefPtr def,
                             const char *reason,
                             bool success);
-void qemuDomainCgroupAudit(virDomainObjPtr vm,
+void qemuAuditCgroup(virDomainObjPtr vm,
-                           virCgroupPtr group,
+                     virCgroupPtr group,
-                           const char *reason,
+                     const char *reason,
-                           const char *item,
+                     const char *extra,
-                           const char *name,
+                     bool success);
-                           bool success);
+void qemuAuditCgroupMajor(virDomainObjPtr vm,
+                          virCgroupPtr group,
+                          const char *reason,
+                          int maj,
+                          const char *name,
+                          bool success);
+void qemuAuditCgroupPath(virDomainObjPtr vm,
+                         virCgroupPtr group,
+                         const char *reason,
+                         const char *path,
+                         int rc);
 void qemuDomainMemoryAudit(virDomainObjPtr vm,
                            unsigned long long oldmem,
                            unsigned long long newmem,

src/qemu/qemu_cgroup.c

@@ -67,9 +67,7 @@ qemuSetupDiskPathAllow(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
     VIR_DEBUG("Process path %s for disk", path);
     /* XXX RO vs RW */
     rc = virCgroupAllowDevicePath(data->cgroup, path);
-    if (rc <= 0)
+    qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
-        qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "path", path,
-                              rc == 0);
     if (rc < 0) {
         if (rc == -EACCES) { /* Get this for root squash NFS */
             VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -110,9 +108,7 @@ qemuTeardownDiskPathDeny(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
     VIR_DEBUG("Process path %s for disk", path);
     /* XXX RO vs RW */
     rc = virCgroupDenyDevicePath(data->cgroup, path);
-    if (rc <= 0)
+    qemuAuditCgroupPath(data->vm, data->cgroup, "deny", path, rc);
-        qemuDomainCgroupAudit(data->vm, data->cgroup, "deny", "path", path,
-                              rc == 0);
     if (rc < 0) {
         if (rc == -EACCES) { /* Get this for root squash NFS */
             VIR_DEBUG("Ignoring EACCES for %s", path);
@@ -155,9 +151,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)
+    qemuAuditCgroupPath(data->vm, data->cgroup, "allow",
-        qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "path",
+                        dev->source.data.file.path, rc);
-                              dev->source.data.file.path, rc == 0);
     if (rc < 0) {
         virReportSystemError(-rc,
                              _("Unable to allow device %s for %s"),
@@ -178,9 +173,7 @@ int qemuSetupHostUsbDeviceCgroup(usbDevice *dev ATTRIBUTE_UNUSED,
 
     VIR_DEBUG("Process path '%s' for USB device", path);
     rc = virCgroupAllowDevicePath(data->cgroup, path);
-    if (rc <= 0)
+    qemuAuditCgroupPath(data->vm, data->cgroup, "allow", path, rc);
-        qemuDomainCgroupAudit(data->vm, data->cgroup, "allow", "path", path,
-                              rc == 0);
     if (rc < 0) {
         virReportSystemError(-rc,
                              _("Unable to allow device %s"),
@@ -216,7 +209,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);
+        qemuAuditCgroup(vm, cgroup, "deny", "all", rc == 0);
         if (rc != 0) {
             if (rc == -EPERM) {
                 VIR_WARN0("Group devices ACL is not accessible, disabling whitelisting");
@@ -234,7 +227,8 @@ int qemuSetupCgroup(struct qemud_driver *driver,
         }
 
         rc = virCgroupAllowDeviceMajor(cgroup, 'c', DEVICE_PTY_MAJOR);
-        qemuDomainCgroupAudit(vm, cgroup, "allow", "major", "pty", rc == 0);
+        qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_PTY_MAJOR,
+                             "pty", rc == 0);
         if (rc != 0) {
             virReportSystemError(-rc, "%s",
                                  _("unable to allow /dev/pts/ devices"));
@@ -247,8 +241,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",
+            qemuAuditCgroupMajor(vm, cgroup, "allow", DEVICE_SND_MAJOR,
-                                  rc == 0);
+                                 "sound", rc == 0);
             if (rc != 0) {
                 virReportSystemError(-rc, "%s",
                                      _("unable to allow /dev/snd/ devices"));
@@ -259,8 +253,7 @@ int qemuSetupCgroup(struct qemud_driver *driver,
         for (i = 0; deviceACL[i] != NULL ; i++) {
             rc = virCgroupAllowDevicePath(cgroup,
                                           deviceACL[i]);
-            qemuDomainCgroupAudit(vm, cgroup, "allow", "path",
+            qemuAuditCgroupPath(vm, cgroup, "allow", deviceACL[i], rc);
-                                  deviceACL[i], rc == 0);
             if (rc < 0 &&
                 rc != -ENOENT) {
                 virReportSystemError(-rc,

src/qemu/qemu_driver.c

@@ -1964,8 +1964,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
             goto endjob;
         }
         rc = virCgroupAllowDevicePath(cgroup, path);
-        if (rc <= 0)
+        qemuAuditCgroupPath(vm, cgroup, "allow", path, rc);
-            qemuDomainCgroupAudit(vm, cgroup, "allow", "path", path, rc == 0);
         if (rc < 0) {
             virReportSystemError(-rc,
                                  _("Unable to allow device %s for %s"),
@@ -2015,8 +2014,7 @@ static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
 
     if (cgroup != NULL) {
         rc = virCgroupDenyDevicePath(cgroup, path);
-        if (rc <= 0)
+        qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
-            qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path, rc == 0);
         if (rc < 0)
             VIR_WARN("Unable to deny device %s for %s %d",
                      path, vm->def->name, rc);
@@ -2048,9 +2046,7 @@ endjob:
 
             if (cgroup != NULL) {
                 rc = virCgroupDenyDevicePath(cgroup, path);
-                if (rc <= 0)
+                qemuAuditCgroupPath(vm, cgroup, "deny", path, rc);
-                    qemuDomainCgroupAudit(vm, cgroup, "deny", "path", path,
-                                          rc == 0);
                 if (rc < 0)
                     VIR_WARN("Unable to deny device %s for %s: %d",
                              path, vm->def->name, rc);