Re: [PATCH v4 4/4] vduse: Add LSM hooks to check Virtio device type
From: Maxime Coquelin
Date: Fri Dec 08 2023 - 07:23:19 EST
On 12/8/23 12:05, Michael S. Tsirkin wrote:
On Fri, Dec 08, 2023 at 12:01:15PM +0100, Maxime Coquelin wrote:
Hello Paul,
On 11/8/23 03:31, Paul Moore wrote:
On Oct 20, 2023 "Michael S. Tsirkin" <mst@xxxxxxxxxx> wrote:
This patch introduces LSM hooks for devices creation,
destruction and opening operations, checking the
application is allowed to perform these operations for
the Virtio device type.
Signed-off-by: Maxime Coquelin <maxime.coquelin@xxxxxxxxxx>
---
drivers/vdpa/vdpa_user/vduse_dev.c | 12 +++++++
include/linux/lsm_hook_defs.h | 4 +++
include/linux/security.h | 15 ++++++++
security/security.c | 42 ++++++++++++++++++++++
security/selinux/hooks.c | 55 +++++++++++++++++++++++++++++
security/selinux/include/classmap.h | 2 ++
6 files changed, 130 insertions(+)
My apologies for the late reply, I've been trying to work my way through
the review backlog but it has been taking longer than expected; comments
below ...
No worries, I have also been busy these days.
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 2aa0e219d721..65d9262a37f7 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -21,6 +21,7 @@
* Copyright (C) 2016 Mellanox Technologies
*/
+#include "av_permissions.h"
#include <linux/init.h>
#include <linux/kd.h>
#include <linux/kernel.h>
@@ -92,6 +93,7 @@
#include <linux/fsnotify.h>
#include <linux/fanotify.h>
#include <linux/io_uring.h>
+#include <uapi/linux/virtio_ids.h>
#include "avc.h"
#include "objsec.h"
@@ -6950,6 +6952,56 @@ static int selinux_uring_cmd(struct io_uring_cmd *ioucmd)
}
#endif /* CONFIG_IO_URING */
+static int vduse_check_device_type(u32 sid, u32 device_id)
+{
+ u32 requested;
+
+ if (device_id == VIRTIO_ID_NET)
+ requested = VDUSE__NET;
+ else if (device_id == VIRTIO_ID_BLOCK)
+ requested = VDUSE__BLOCK;
+ else
+ return -EINVAL;
+
+ return avc_has_perm(sid, sid, SECCLASS_VDUSE, requested, NULL);
+}
+
+static int selinux_vduse_dev_create(u32 device_id)
+{
+ u32 sid = current_sid();
+ int ret;
+
+ ret = avc_has_perm(sid, sid, SECCLASS_VDUSE, VDUSE__DEVCREATE, NULL);
+ if (ret)
+ return ret;
+
+ return vduse_check_device_type(sid, device_id);
+}
I see there has been some discussion about the need for a dedicated
create hook as opposed to using the existing ioctl controls. I think
one important point that has been missing from the discussion is the
idea of labeling the newly created device. Unfortunately prior to a
few minutes ago I hadn't ever looked at VDUSE so please correct me if
I get some things wrong :)
From what I can see userspace creates a new VDUSE device with
ioctl(VDUSE_CREATE_DEV), which trigger the creation of a new
/dev/vduse/XXX device which will be labeled according to the udev
and SELinux configuration, likely with a generic udev label. My
question is if we want to be able to uniquely label each VDUSE
device based on the process that initiates the device creation
with the call to ioctl()? If that is the case, we would need a
create hook not only to control the creation of the device, but to
record the triggering process' label in the new device; this label
would then be used in subsequent VDUSE open and destroy operations.
The normal device file I/O operations would still be subject to the
standard SELinux file I/O permissions using the device file label
assigned by systemd/udev when the device was created.
I don't think we need a unique label for VDUSE devices, but maybe
Michael thinks otherwise?
I don't know.
All this is consumed by libvirt, you need to ask these guys.
I think it is not consumed by libvirt, at least not in the usecases I
have in mind. For networking devices, it will be consumed by OVS.
Maxime