89ddbf
From: Javier Martinez Canillas <javierm@redhat.com>
89ddbf
Subject: [PATCH v2] efi/efi_test: lock down /dev/efi_test and require
89ddbf
 CAP_SYS_ADMIN
89ddbf
Date: Tue,  8 Oct 2019 12:55:10 +0200
89ddbf
89ddbf
The driver exposes EFI runtime services to user-space through an IOCTL
89ddbf
interface, calling the EFI services function pointers directly without
89ddbf
using the efivar API.
89ddbf
89ddbf
Disallow access to the /dev/efi_test character device when the kernel is
89ddbf
locked down to prevent arbitrary user-space to call EFI runtime services.
89ddbf
89ddbf
Also require CAP_SYS_ADMIN to open the chardev to prevent unprivileged
89ddbf
users to call the EFI runtime services, instead of just relying on the
89ddbf
chardev file mode bits for this.
89ddbf
89ddbf
The main user of this driver is the fwts [0] tool that already checks if
89ddbf
the effective user ID is 0 and fails otherwise. So this change shouldn't
89ddbf
cause any regression to this tool.
89ddbf
89ddbf
[0]: https://wiki.ubuntu.com/FirmwareTestSuite/Reference/uefivarinfo
89ddbf
89ddbf
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
89ddbf
Acked-by: Laszlo Ersek <lersek@redhat.com>
89ddbf
Acked-by: Matthew Garrett <mjg59@google.com>
89ddbf
---
89ddbf
89ddbf
Changes in v2:
89ddbf
- Also disable /dev/efi_test access when the kernel is locked down as
89ddbf
  suggested by Matthew Garrett.
89ddbf
- Add Acked-by tag from Laszlo Ersek.
89ddbf
89ddbf
 drivers/firmware/efi/test/efi_test.c | 8 ++++++++
89ddbf
 include/linux/security.h             | 1 +
89ddbf
 security/lockdown/lockdown.c         | 1 +
89ddbf
 3 files changed, 10 insertions(+)
89ddbf
89ddbf
diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
89ddbf
index 877745c3aaf..7baf48c01e7 100644
89ddbf
--- a/drivers/firmware/efi/test/efi_test.c
89ddbf
+++ b/drivers/firmware/efi/test/efi_test.c
89ddbf
@@ -14,6 +14,7 @@
89ddbf
 #include <linux/init.h>
89ddbf
 #include <linux/proc_fs.h>
89ddbf
 #include <linux/efi.h>
89ddbf
+#include <linux/security.h>
89ddbf
 #include <linux/slab.h>
89ddbf
 #include <linux/uaccess.h>
89ddbf
 
89ddbf
@@ -717,6 +718,13 @@ static long efi_test_ioctl(struct file *file, unsigned int cmd,
89ddbf
 
89ddbf
 static int efi_test_open(struct inode *inode, struct file *file)
89ddbf
 {
89ddbf
+	int ret = security_locked_down(LOCKDOWN_EFI_TEST);
89ddbf
+
89ddbf
+	if (ret)
89ddbf
+		return ret;
89ddbf
+
89ddbf
+	if (!capable(CAP_SYS_ADMIN))
89ddbf
+		return -EACCES;
89ddbf
 	/*
89ddbf
 	 * nothing special to do here
89ddbf
 	 * We do accept multiple open files at the same time as we
89ddbf
diff --git a/include/linux/security.h b/include/linux/security.h
89ddbf
index a8d59d612d2..9df7547afc0 100644
89ddbf
--- a/include/linux/security.h
89ddbf
+++ b/include/linux/security.h
89ddbf
@@ -105,6 +105,7 @@ enum lockdown_reason {
89ddbf
 	LOCKDOWN_NONE,
89ddbf
 	LOCKDOWN_MODULE_SIGNATURE,
89ddbf
 	LOCKDOWN_DEV_MEM,
89ddbf
+	LOCKDOWN_EFI_TEST,
89ddbf
 	LOCKDOWN_KEXEC,
89ddbf
 	LOCKDOWN_HIBERNATION,
89ddbf
 	LOCKDOWN_PCI_ACCESS,
89ddbf
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
89ddbf
index 8a10b43daf7..40b790536de 100644
89ddbf
--- a/security/lockdown/lockdown.c
89ddbf
+++ b/security/lockdown/lockdown.c
89ddbf
@@ -20,6 +20,7 @@ static const char *const lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
89ddbf
 	[LOCKDOWN_NONE] = "none",
89ddbf
 	[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
89ddbf
 	[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
89ddbf
+	[LOCKDOWN_EFI_TEST] = "/dev/efi_test access",
89ddbf
 	[LOCKDOWN_KEXEC] = "kexec of unsigned images",
89ddbf
 	[LOCKDOWN_HIBERNATION] = "hibernation",
89ddbf
 	[LOCKDOWN_PCI_ACCESS] = "direct PCI access",