[1/1] Revert "UBUNTU: SAUCE: (lockdown) Add a SysRq option to lift kernel lockdown"
diff mbox series

Message ID 20200207203926.9802-2-tyhicks@canonical.com
State New
Headers show
Series
  • Root can lift kernel lockdown via USB/IP (LP: #1861238)
Related show

Commit Message

Tyler Hicks Feb. 7, 2020, 8:39 p.m. UTC
BugLink: https://bugs.launchpad.net/bugs/1861238

This reverts commit de4a78d3b642e5504b28e901b07eb4da6784dd3d.

The original intent behind Lockdown's SysRq support was that the SysRq
command to lift Lockdown would only be honored if the command was
physically entered on a keyboard. Attempts to synthetically generate the
SysRq command, by a software program, were to be ignored since software,
even running as root, must not have the authorization to lift Lockdown.

Unfortunately, attempts to detect a synthetic SysRq command can be
thwarted by a privileged process that is able to set up a USB/IP
connection as the USB/IP connection could be used to lift Lockdown.

Remove the ability to lift Lockdown using SysRq.

Signed-off-by: Tyler Hicks <tyhicks@canonical.com>
---
 arch/x86/include/asm/setup.h              |  2 --
 debian.master/config/annotations          |  2 --
 debian.master/config/config.common.ubuntu |  1 -
 drivers/input/misc/uinput.c               |  1 -
 drivers/tty/sysrq.c                       | 27 ++++++--------
 include/linux/input.h                     |  5 ---
 include/linux/sysrq.h                     |  8 +----
 kernel/debug/kdb/kdb_main.c               |  2 +-
 security/lockdown/Kconfig                 | 11 ------
 security/lockdown/lockdown.c              | 43 -----------------------
 10 files changed, 12 insertions(+), 90 deletions(-)

Comments

Paolo Pisati Feb. 17, 2020, 4:33 p.m. UTC | #1
On Fri, Feb 07, 2020 at 08:39:26PM +0000, Tyler Hicks wrote:
> BugLink: https://bugs.launchpad.net/bugs/1861238
> 
> This reverts commit de4a78d3b642e5504b28e901b07eb4da6784dd3d.
> 
> The original intent behind Lockdown's SysRq support was that the SysRq
> command to lift Lockdown would only be honored if the command was
> physically entered on a keyboard. Attempts to synthetically generate the
> SysRq command, by a software program, were to be ignored since software,
> even running as root, must not have the authorization to lift Lockdown.
> 
> Unfortunately, attempts to detect a synthetic SysRq command can be
> thwarted by a privileged process that is able to set up a USB/IP
> connection as the USB/IP connection could be used to lift Lockdown.
> 
> Remove the ability to lift Lockdown using SysRq.
> 
> Signed-off-by: Tyler Hicks <tyhicks@canonical.com>

Applied to Focal/linux

Patch
diff mbox series

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index 8daf633a5347..ed8ec011a9fd 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -9,8 +9,6 @@ 
 #include <linux/linkage.h>
 #include <asm/page_types.h>
 
-#define LOCKDOWN_LIFT_KEY 'x'
-
 #ifdef __i386__
 
 #include <linux/pfn.h>
diff --git a/debian.master/config/annotations b/debian.master/config/annotations
index e70e9f2705c2..2c5feaeca830 100644
--- a/debian.master/config/annotations
+++ b/debian.master/config/annotations
@@ -12820,12 +12820,10 @@  CONFIG_SECURITY_SAFESETID                       mark<ENFORCED> note<LP:#1845391>
 CONFIG_SECURITY_LOCKDOWN_LSM                    policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
 CONFIG_SECURITY_LOCKDOWN_LSM_EARLY              policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
 CONFIG_LOCK_DOWN_IN_SECURE_BOOT                 policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 's390x': 'y'}>
-CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ             policy<{'amd64': 'y', 'i386': 'y'}>
 #
 CONFIG_SECURITY_LOCKDOWN_LSM                    mark<ENFORCED>
 CONFIG_SECURITY_LOCKDOWN_LSM_EARLY              mark<ENFORCED>
 CONFIG_LOCK_DOWN_IN_SECURE_BOOT                 mark<ENFORCED>
-CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ             mark<ENFORCED>
 
 # Menu: Security options >> Enable different security models >> Basic module for enforcing kernel lockdown >> Kernel default lockdown mode
 CONFIG_LOCK_DOWN_KERNEL_FORCE_NONE              policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'y', 's390x': 'y'}>
diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu
index 19530e384f53..6f23affd9e81 100644
--- a/debian.master/config/config.common.ubuntu
+++ b/debian.master/config/config.common.ubuntu
@@ -252,7 +252,6 @@  CONFIG_ALIGNMENT_TRAP=y
 CONFIG_ALIM1535_WDT=m
 CONFIG_ALIX=y
 CONFIG_ALLOW_DEV_COREDUMP=y
-CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ=y
 CONFIG_ALPINE_MSI=y
 CONFIG_ALTERA_FREEZE_BRIDGE=m
 CONFIG_ALTERA_MBOX=m
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 5a50e13be4e2..002654ec7040 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -357,7 +357,6 @@  static int uinput_create_device(struct uinput_device *udev)
 		dev->flush = uinput_dev_flush;
 	}
 
-	dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
 	dev->event = uinput_dev_event;
 
 	input_set_drvdata(udev->dev, udev);
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 99082faafc44..573b2055173c 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -480,7 +480,6 @@  static struct sysrq_key_op *sysrq_key_table[36] = {
 	/* x: May be registered on mips for TLB dump */
 	/* x: May be registered on ppc/powerpc for xmon */
 	/* x: May be registered on sparc64 for global PMU dump */
-	/* x: May be registered on x86_64 for disabling secure boot */
 	NULL,				/* x */
 	/* y: May be registered on sparc64 for global register dump */
 	NULL,				/* y */
@@ -524,7 +523,7 @@  static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
                 sysrq_key_table[i] = op_p;
 }
 
-void __handle_sysrq(int key, unsigned int from)
+void __handle_sysrq(int key, bool check_mask)
 {
 	struct sysrq_key_op *op_p;
 	int orig_log_level;
@@ -547,15 +546,11 @@  void __handle_sysrq(int key, unsigned int from)
 
         op_p = __sysrq_get_key_op(key);
         if (op_p) {
-		/* Ban synthetic events from some sysrq functionality */
-		if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
-		    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
-			printk("This sysrq operation is disabled from userspace.\n");
-		} else if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
-			/*
-			 * Should we check for enabled operations (/proc/sysrq-trigger
-			 * should not) and is the invoked operation enabled?
-			 */
+		/*
+		 * Should we check for enabled operations (/proc/sysrq-trigger
+		 * should not) and is the invoked operation enabled?
+		 */
+		if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
 			pr_info("%s\n", op_p->action_msg);
 			console_loglevel = orig_log_level;
 			op_p->handler(key);
@@ -590,7 +585,7 @@  void __handle_sysrq(int key, unsigned int from)
 void handle_sysrq(int key)
 {
 	if (sysrq_on())
-		__handle_sysrq(key, SYSRQ_FROM_KERNEL);
+		__handle_sysrq(key, true);
 }
 EXPORT_SYMBOL(handle_sysrq);
 
@@ -670,7 +665,7 @@  static void sysrq_do_reset(struct timer_list *t)
 static void sysrq_handle_reset_request(struct sysrq_state *state)
 {
 	if (state->reset_requested)
-		__handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);
+		__handle_sysrq(sysrq_xlate[KEY_B], false);
 
 	if (sysrq_reset_downtime_ms)
 		mod_timer(&state->keyreset_timer,
@@ -823,10 +818,8 @@  static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
 
 	default:
 		if (sysrq->active && value && value != 2) {
-			int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
-					SYSRQ_FROM_SYNTHETIC : 0;
 			sysrq->need_reinject = false;
-			__handle_sysrq(sysrq_xlate[code], from);
+			__handle_sysrq(sysrq_xlate[code], true);
 		}
 		break;
 	}
@@ -1109,7 +1102,7 @@  static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
 
 		if (get_user(c, buf))
 			return -EFAULT;
-		__handle_sysrq(c, SYSRQ_FROM_PROC);
+		__handle_sysrq(c, false);
 	}
 
 	return count;
diff --git a/include/linux/input.h b/include/linux/input.h
index 8539afa2c001..94f277cd806a 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -48,7 +48,6 @@  enum input_clock_type {
  * @phys: physical path to the device in the system hierarchy
  * @uniq: unique identification code for the device (if device has it)
  * @id: id of the device (struct input_id)
- * @flags: input device flags (SYNTHETIC, etc.)
  * @propbit: bitmap of device properties and quirks
  * @evbit: bitmap of types of events supported by the device (EV_KEY,
  *	EV_REL, etc.)
@@ -135,8 +134,6 @@  struct input_dev {
 	const char *uniq;
 	struct input_id id;
 
-	unsigned int flags;
-
 	unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];
 
 	unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
@@ -207,8 +204,6 @@  struct input_dev {
 };
 #define to_input_dev(d) container_of(d, struct input_dev, dev)
 
-#define	INPUTDEV_FLAGS_SYNTHETIC	0x000000001
-
 /*
  * Verify that we are in sync with input_device_id mod_devicetable.h #defines
  */
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 7de1f08b60a9..8c71874e8485 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -29,8 +29,6 @@ 
 #define SYSRQ_ENABLE_BOOT	0x0080
 #define SYSRQ_ENABLE_RTNICE	0x0100
 
-#define SYSRQ_DISABLE_USERSPACE	0x00010000
-
 struct sysrq_key_op {
 	void (*handler)(int);
 	char *help_msg;
@@ -45,12 +43,8 @@  struct sysrq_key_op {
  * are available -- else NULL's).
  */
 
-#define SYSRQ_FROM_KERNEL	0x0001
-#define SYSRQ_FROM_PROC		0x0002
-#define SYSRQ_FROM_SYNTHETIC	0x0004
-
 void handle_sysrq(int key);
-void __handle_sysrq(int key, unsigned int from);
+void __handle_sysrq(int key, bool check_mask);
 int register_sysrq_key(int key, struct sysrq_key_op *op);
 int unregister_sysrq_key(int key, struct sysrq_key_op *op);
 struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index d05142ef44c4..4567fe998c30 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1981,7 +1981,7 @@  static int kdb_sr(int argc, const char **argv)
 		return KDB_ARGCOUNT;
 
 	kdb_trap_printk++;
-	__handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
+	__handle_sysrq(*argv[1], check_mask);
 	kdb_trap_printk--;
 
 	return 0;
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
index 77a61130f90b..e508c99a6607 100644
--- a/security/lockdown/Kconfig
+++ b/security/lockdown/Kconfig
@@ -58,14 +58,3 @@  config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
 	 disabled.
 
 endchoice
-
-config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
-	bool "Allow the kernel lockdown to be lifted by SysRq"
-    depends on SECURITY_LOCKDOWN_LSM
-    depends on !LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
-    depends on !LOCK_DOWN_KERNEL_FORCE_INTEGRITY
-    depends on MAGIC_SYSRQ
-    depends on X86
-	help
-      Allow setting the lockdown mode to "none" by pressing a SysRq key
-      combination on a wired keyboard. On x86, this is SysRq+x
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 95c103fc5b03..92b2729cb20a 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -13,8 +13,6 @@ 
 #include <linux/security.h>
 #include <linux/export.h>
 #include <linux/lsm_hooks.h>
-#include <linux/sysrq.h>
-#include <asm/setup.h>
 
 static enum lockdown_reason kernel_locked_down;
 
@@ -183,47 +181,6 @@  static int __init lockdown_secfs_init(void)
 	return PTR_ERR_OR_ZERO(dentry);
 }
 
-#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
-
-/*
- * Take the kernel out of lockdown mode.
- */
-static void lift_kernel_lockdown(void)
-{
-	pr_notice("Lifting lockdown\n");
-	kernel_locked_down = LOCKDOWN_NONE;
-}
-
-/*
- * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
- * echoing the appropriate letter into the sysrq-trigger file).
- */
-static void sysrq_handle_lockdown_lift(int key)
-{
-	if (kernel_locked_down)
-		lift_kernel_lockdown();
-}
-
-static struct sysrq_key_op lockdown_lift_sysrq_op = {
-	.handler	= sysrq_handle_lockdown_lift,
-	.help_msg	= "unSB(x)",
-	.action_msg	= "Disabling Secure Boot restrictions",
-	.enable_mask	= SYSRQ_DISABLE_USERSPACE,
-};
-
-static int __init lockdown_lift_sysrq(void)
-{
-	if (kernel_locked_down) {
-		lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
-		register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
-	}
-	return 0;
-}
-
-late_initcall(lockdown_lift_sysrq);
-
-#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
-
 core_initcall(lockdown_secfs_init);
 
 #ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY