From patchwork Fri Feb 7 20:40:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tyler Hicks X-Patchwork-Id: 1235134 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=canonical.com Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 48DnJr2MjZz9sRm; Sat, 8 Feb 2020 07:41:32 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1j0ARN-0005Ly-6v; Fri, 07 Feb 2020 20:41:29 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1j0ARK-0005LY-KM for kernel-team@lists.ubuntu.com; Fri, 07 Feb 2020 20:41:26 +0000 Received: from 2.general.tyhicks.us.vpn ([10.172.64.53] helo=sec.work.tihix.com) by youngberry.canonical.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1j0ARK-0008Rr-0y; Fri, 07 Feb 2020 20:41:26 +0000 From: Tyler Hicks To: kernel-team@lists.ubuntu.com Subject: [PATCH 1/1] Revert "UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel lockdown" Date: Fri, 7 Feb 2020 20:40:59 +0000 Message-Id: <20200207204059.9969-2-tyhicks@canonical.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20200207204059.9969-1-tyhicks@canonical.com> References: <20200207204059.9969-1-tyhicks@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" BugLink: https://bugs.launchpad.net/bugs/1861238 This reverts commit b2bd7140610d52ed7460522c45bb4caf80b79aa6. 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 --- 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/Kconfig | 10 ----- security/lock_down.c | 47 ----------------------- 10 files changed, 12 insertions(+), 93 deletions(-) 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 #include -#define LOCKDOWN_LIFT_KEY 'x' - #ifdef __i386__ #include diff --git a/debian.master/config/annotations b/debian.master/config/annotations index 947286c1ebb3..b95e91cb2218 100644 --- a/debian.master/config/annotations +++ b/debian.master/config/annotations @@ -12205,11 +12205,9 @@ CONFIG_FORTIFY_SOURCE policy<{'amd64': 'y', 'arm64': ' CONFIG_STATIC_USERMODEHELPER policy<{'amd64': 'n', 'arm64': 'n', 'armhf': 'n', 'i386': 'n', 'ppc64el': 'n', 's390x': 'n'}> CONFIG_LOCK_DOWN_KERNEL policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y', 'ppc64el': 'n', 's390x': 'n'}> CONFIG_LOCK_DOWN_MANDATORY policy<{'amd64': 'n', 'arm64': 'n', 'armhf': 'n', 'i386': 'n'}> -CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ policy<{'amd64': 'y', 'i386': 'y'}> CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT policy<{'amd64': 'y', 'arm64': 'y', 'armhf': 'y', 'i386': 'y'}> CONFIG_LSM policy<{'amd64': '"yama,loadpin,integrity,apparmor"', 'arm64-generic': '"yama,loadpin,integrity,apparmor"', 'armhf': '"yama,loadpin,integrity,apparmor"', 'i386': '"yama,loadpin,integrity,apparmor"', 'ppc64el': '"yama,loadpin,integrity,apparmor"', 's390x': '"yama,loadpin,integrity,apparmor"'}> # -CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ mark CONFIG_LOCK_DOWN_IN_EFI_SECURE_BOOT mark CONFIG_LOCK_DOWN_KERNEL mark flag CONFIG_LSM mark diff --git a/debian.master/config/config.common.ubuntu b/debian.master/config/config.common.ubuntu index 7e5fb52a9fe9..a5afa29592b9 100644 --- a/debian.master/config/config.common.ubuntu +++ b/debian.master/config/config.common.ubuntu @@ -246,7 +246,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_MBOX=m CONFIG_ALTERA_MSGDMA=m diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c index f91800b93458..83d1499fe021 100644 --- a/drivers/input/misc/uinput.c +++ b/drivers/input/misc/uinput.c @@ -366,7 +366,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 cbaf2e5a8cb8..1f03078ec352 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; @@ -544,15 +543,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_cont("%s\n", op_p->action_msg); console_loglevel = orig_log_level; op_p->handler(key); @@ -584,7 +579,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); @@ -664,7 +659,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, @@ -817,10 +812,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; } @@ -1103,7 +1096,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 38cd0ea72c37..7c7516eb7d76 100644 --- a/include/linux/input.h +++ b/include/linux/input.h @@ -42,7 +42,6 @@ struct input_value { * @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.) @@ -125,8 +124,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)]; @@ -193,8 +190,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 efee1abf5e8e..82a3b32a7cfc 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/Kconfig b/security/Kconfig index 48a727cd514b..439888ffe17c 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -260,16 +260,6 @@ config LOCK_DOWN_MANDATORY Makes the lockdown non-negotiable. It is always on and cannot be disabled. -config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ - bool "Allow the kernel lockdown to be lifted by SysRq" - depends on LOCK_DOWN_KERNEL - depends on !LOCK_DOWN_MANDATORY - depends on MAGIC_SYSRQ - depends on X86 - help - Allow the lockdown on a kernel to be lifted, by pressing a SysRq key - combination on a wired keyboard. On x86, this is SysRq+x. - config LOCK_DOWN_IN_EFI_SECURE_BOOT bool "Lock down the kernel in EFI Secure Boot mode" default n diff --git a/security/lock_down.c b/security/lock_down.c index e3ef9e2b4dd1..cb07578627f6 100644 --- a/security/lock_down.c +++ b/security/lock_down.c @@ -11,16 +11,10 @@ #include #include -#include #include -#include #ifndef CONFIG_LOCK_DOWN_MANDATORY -#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ -static __read_mostly bool kernel_locked_down; -#else static __ro_after_init bool kernel_locked_down; -#endif #else #define kernel_locked_down true #endif @@ -74,44 +68,3 @@ bool __kernel_is_locked_down(const char *what, bool first) return kernel_locked_down; } EXPORT_SYMBOL(__kernel_is_locked_down); - -#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 = false; -} - -/* - * 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 */