[libvirt] [PATCH 1/2] security: aa-helper: nvidia rules for gl devices
Further testing with different devices showed that we need more rules to drive gl backends with nvidia cards. Related denies look like: apparmor="DENIED" operation="open" name="/usr/share/egl/egl_external_platform.d/" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/modules" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/driver/nvidia/params" requested_mask="r" apparmor="DENIED" operation="mknod" name="/dev/nvidiactl" requested_mask="c" Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e9120213ff..13b507ff69 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1279,6 +1279,11 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/usr/share/drirc.d/{,*.conf}\" r,\n"); virBufferAddLit(&buf, " \"/etc/glvnd/egl_vendor.d/{,*}\" r,\n"); virBufferAddLit(&buf, " \"/usr/share/glvnd/egl_vendor.d/{,*}\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/*\" r,\n"); + virBufferAddLit(&buf, " \"/proc/modules\" r,\n"); + virBufferAddLit(&buf, " \"/proc/driver/nvidia/params\" r,\n"); + virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n"); virBufferAddLit(&buf, " # Probe DRI device attributes\n"); virBufferAddLit(&buf, " \"/dev/dri/\" r,\n"); virBufferAddLit(&buf, " \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); -- 2.17.1
Further testing with more devices showed that we sometimes have a different depth of pci device paths when accessing sysfs for device attributes. But since the access is limited to a set of filenames and read only it is safe to use a wildcard for that. Related apparmor denies - while we formerly had only considered: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:02.1/uevent" requested_mask="r" We now also know of cases like: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/uevent" requested_mask="r" Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943 Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 13b507ff69..989dcf1784 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1286,8 +1286,7 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n"); virBufferAddLit(&buf, " # Probe DRI device attributes\n"); virBufferAddLit(&buf, " \"/dev/dri/\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); + virBufferAddLit(&buf, " \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); virBufferAddLit(&buf, " # dri libs will trigger that, but t is not requited and DAC would deny it anyway\n"); virBufferAddLit(&buf, " deny \"/var/lib/libvirt/.cache/\" w,\n"); } -- 2.17.1
On Tue, 05 Mar 2019, Christian Ehrhardt wrote:
Further testing with more devices showed that we sometimes have a different depth of pci device paths when accessing sysfs for device attributes.
But since the access is limited to a set of filenames and read only it is safe to use a wildcard for that.
Related apparmor denies - while we formerly had only considered: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:02.1/uevent" requested_mask="r"
We now also know of cases like: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/uevent" requested_mask="r"
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 13b507ff69..989dcf1784 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1286,8 +1286,7 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n"); virBufferAddLit(&buf, " # Probe DRI device attributes\n"); virBufferAddLit(&buf, " \"/dev/dri/\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); + virBufferAddLit(&buf, " \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
I'm curious about the new denials, but the reads for these files should be fine. -- Jamie Strandboge | http://www.canonical.com
On Tue, Mar 5, 2019 at 5:48 PM Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 05 Mar 2019, Christian Ehrhardt wrote:
Further testing with more devices showed that we sometimes have a different depth of pci device paths when accessing sysfs for device attributes.
But since the access is limited to a set of filenames and read only it is safe to use a wildcard for that.
Related apparmor denies - while we formerly had only considered: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:02.1/uevent" requested_mask="r"
We now also know of cases like: apparmor="DENIED" operation="open" name="/sys/devices/pci0000:00/0000:00:03.1/0000:1c:00.0/uevent" requested_mask="r"
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index 13b507ff69..989dcf1784 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1286,8 +1286,7 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n"); virBufferAddLit(&buf, " # Probe DRI device attributes\n"); virBufferAddLit(&buf, " \"/dev/dri/\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); - virBufferAddLit(&buf, " \"/sys/devices/*/*/drm/*/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n"); + virBufferAddLit(&buf, " \"/sys/devices/**/{uevent,vendor,device,subsystem_vendor,subsystem_device}\" r,\n");
I'm curious about the new denials, but the reads for these files should be fine.
Yes it is odd, but as it seems to be HW dependent that seems to be the only way covers what is needed and seems safe. Thanks for reviewing - Pushed with your ack
-- Jamie Strandboge | http://www.canonical.com
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
On Tue, 05 Mar 2019, Christian Ehrhardt wrote:
Further testing with different devices showed that we need more rules to drive gl backends with nvidia cards. Related denies look like:
apparmor="DENIED" operation="open" name="/usr/share/egl/egl_external_platform.d/" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/modules" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/driver/nvidia/params" requested_mask="r" apparmor="DENIED" operation="mknod" name="/dev/nvidiactl" requested_mask="c"
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e9120213ff..13b507ff69 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1279,6 +1279,11 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/usr/share/drirc.d/{,*.conf}\" r,\n"); virBufferAddLit(&buf, " \"/etc/glvnd/egl_vendor.d/{,*}\" r,\n"); virBufferAddLit(&buf, " \"/usr/share/glvnd/egl_vendor.d/{,*}\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/*\" r,\n"); + virBufferAddLit(&buf, " \"/proc/modules\" r,\n"); + virBufferAddLit(&buf, " \"/proc/driver/nvidia/params\" r,\n"); + virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n");
All the reads are fine. The 'rw' for nvidiactl is unfortunate but there isn't anything we can do about the need for it. At least the policy doesn't have 'capability mknod' and DAC will protect against creating/removing the device where the VMs run as non-root. +1 to apply -- Jamie Strandboge | http://www.canonical.com
On Tue, Mar 5, 2019 at 5:45 PM Jamie Strandboge <jamie@canonical.com> wrote:
On Tue, 05 Mar 2019, Christian Ehrhardt wrote:
Further testing with different devices showed that we need more rules to drive gl backends with nvidia cards. Related denies look like:
apparmor="DENIED" operation="open" name="/usr/share/egl/egl_external_platform.d/" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/modules" requested_mask="r" apparmor="DENIED" operation="open" name="/proc/driver/nvidia/params" requested_mask="r" apparmor="DENIED" operation="mknod" name="/dev/nvidiactl" requested_mask="c"
Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1817943
Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com> --- src/security/virt-aa-helper.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index e9120213ff..13b507ff69 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -1279,6 +1279,11 @@ get_files(vahControl * ctl) virBufferAddLit(&buf, " \"/usr/share/drirc.d/{,*.conf}\" r,\n"); virBufferAddLit(&buf, " \"/etc/glvnd/egl_vendor.d/{,*}\" r,\n"); virBufferAddLit(&buf, " \"/usr/share/glvnd/egl_vendor.d/{,*}\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/\" r,\n"); + virBufferAddLit(&buf, " \"/usr/share/egl/egl_external_platform.d/*\" r,\n"); + virBufferAddLit(&buf, " \"/proc/modules\" r,\n"); + virBufferAddLit(&buf, " \"/proc/driver/nvidia/params\" r,\n"); + virBufferAddLit(&buf, " \"/dev/nvidiactl\" rw,\n");
All the reads are fine. The 'rw' for nvidiactl is unfortunate but there isn't anything we can do about the need for it. At least the policy doesn't have 'capability mknod' and DAC will protect against creating/removing the device where the VMs run as non-root.
+1 to apply
Thanks, pushed with your ack
-- Jamie Strandboge | http://www.canonical.com
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
participants (2)
-
Christian Ehrhardt -
Jamie Strandboge