[PATCH 0/2][SRU][B/D/E] Don't allow lifting of lockdown via /proc/sysrq-trigger

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][SRU][B/D/E] Don't allow lifting of lockdown via /proc/sysrq-trigger

Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1851380

SRU Justification

Impact: The kernel lockdown support adds a sysrq to allow a physically
present user to disable lockdown from the keyboard. A bug in the
implementation makes it possible to also lift lockdown by writing to
/proc/sysrq-trigger.

Fix: Correct the logic to disallow disabling lockdown via
/proc/sysrq-trigger.

Test Case: Write "x" to /proc/sysrq-trigger. When working properly there
should be no messages in dmesg about lifting lockdown, and lockdown
restrictions (e.g. loading unsigned modules) should remain in effect.

Regression Potential: Anyone using /proc/sysrq-trigger to disable
lockdown will no longer be able to do so. Implementation bugs could
prevent use of the sysrq from the keyboard from disabling lockdown, but
this has been confrimed to still work with the fix in place.

Note that xenial uses an older implementation of these patches which
does not have any sysrq mechanism for lifting lockdown, and thus it is
not affected.

Thanks,
Seth

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/1][SRU][B/D] UBUNTU: SAUCE: (efi-lockdown) Really don't allow lifting lockdown from userspace

Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1851380

"UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
lockdown" adds a sysrq key to lift kernel lockdown, which is
meant to only allow a physically present user to lift lockdown
using a keyboard. However, the code has a bug which also allows
root to lift lockdown through /proc/sysrq-trigger. Fix this bug
to make this work as intended.

Signed-off-by: Seth Forshee <[hidden email]>
---
 drivers/tty/sysrq.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 7c06541b422e..f72003937717 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -553,13 +553,13 @@ void __handle_sysrq(int key, unsigned int from)
         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)
+    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
  printk("This sysrq operation is disabled from userspace.\n");
- /*
- * Should we check for enabled operations (/proc/sysrq-trigger
- * should not) and is the invoked operation enabled?
- */
- if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
+ } 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?
+ */
  pr_cont("%s\n", op_p->action_msg);
  console_loglevel = orig_log_level;
  op_p->handler(key);
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/2][SRU][E] UBUNTU: SAUCE: (efi-lockdown) Really don't allow lifting lockdown from userspace

Seth Forshee
In reply to this post by Seth Forshee
BugLink: https://bugs.launchpad.net/bugs/1851380

"UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
lockdown" adds a sysrq key to lift kernel lockdown, which is
meant to only allow a physically present user to lift lockdown
using a keyboard. However, the code has a bug which also allows
root to lift lockdown through /proc/sysrq-trigger. Fix this bug
to make this work as intended.

Signed-off-by: Seth Forshee <[hidden email]>
---
 drivers/tty/sysrq.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 7cc95a8bdf8d..99082faafc44 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -549,13 +549,13 @@ void __handle_sysrq(int key, unsigned int from)
         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)
+    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
  printk("This sysrq operation is disabled from userspace.\n");
- /*
- * Should we check for enabled operations (/proc/sysrq-trigger
- * should not) and is the invoked operation enabled?
- */
- if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
+ } 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?
+ */
  pr_info("%s\n", op_p->action_msg);
  console_loglevel = orig_log_level;
  op_p->handler(key);
--
2.20.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/1][SRU][B/D] UBUNTU: SAUCE: (efi-lockdown) Really don't allow lifting lockdown from userspace

Tyler Hicks-2
In reply to this post by Seth Forshee
On 2019-11-05 14:35:04, Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1851380
>
> "UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
> lockdown" adds a sysrq key to lift kernel lockdown, which is
> meant to only allow a physically present user to lift lockdown
> using a keyboard. However, the code has a bug which also allows
> root to lift lockdown through /proc/sysrq-trigger. Fix this bug
> to make this work as intended.
>
> Signed-off-by: Seth Forshee <[hidden email]>

Acked-by: Tyler Hicks <[hidden email]>

Tyler

> ---
>  drivers/tty/sysrq.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 7c06541b422e..f72003937717 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -553,13 +553,13 @@ void __handle_sysrq(int key, unsigned int from)
>          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)
> +    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
>   printk("This sysrq operation is disabled from userspace.\n");
> - /*
> - * Should we check for enabled operations (/proc/sysrq-trigger
> - * should not) and is the invoked operation enabled?
> - */
> - if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> + } 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?
> + */
>   pr_cont("%s\n", op_p->action_msg);
>   console_loglevel = orig_log_level;
>   op_p->handler(key);
> --
> 2.20.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 2/2][SRU][E] UBUNTU: SAUCE: (efi-lockdown) Really don't allow lifting lockdown from userspace

Tyler Hicks-2
In reply to this post by Seth Forshee
On 2019-11-05 14:35:05, Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1851380
>
> "UBUNTU: SAUCE: (efi-lockdown) Add a SysRq option to lift kernel
> lockdown" adds a sysrq key to lift kernel lockdown, which is
> meant to only allow a physically present user to lift lockdown
> using a keyboard. However, the code has a bug which also allows
> root to lift lockdown through /proc/sysrq-trigger. Fix this bug
> to make this work as intended.
>
> Signed-off-by: Seth Forshee <[hidden email]>

Acked-by: Tyler Hicks <[hidden email]>

Tyler

> ---
>  drivers/tty/sysrq.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 7cc95a8bdf8d..99082faafc44 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -549,13 +549,13 @@ void __handle_sysrq(int key, unsigned int from)
>          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)
> +    op_p->enable_mask & SYSRQ_DISABLE_USERSPACE) {
>   printk("This sysrq operation is disabled from userspace.\n");
> - /*
> - * Should we check for enabled operations (/proc/sysrq-trigger
> - * should not) and is the invoked operation enabled?
> - */
> - if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> + } 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?
> + */
>   pr_info("%s\n", op_p->action_msg);
>   console_loglevel = orig_log_level;
>   op_p->handler(key);
> --
> 2.20.1
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [PATCH 0/2][SRU][B/D/E] Don't allow lifting of lockdown via /proc/sysrq-trigger

Connor Kuehl
In reply to this post by Seth Forshee
On 11/5/19 12:35 PM, Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1851380
>
> SRU Justification
>
> Impact: The kernel lockdown support adds a sysrq to allow a physically
> present user to disable lockdown from the keyboard. A bug in the
> implementation makes it possible to also lift lockdown by writing to
> /proc/sysrq-trigger.
>
> Fix: Correct the logic to disallow disabling lockdown via
> /proc/sysrq-trigger.
>
> Test Case: Write "x" to /proc/sysrq-trigger. When working properly there
> should be no messages in dmesg about lifting lockdown, and lockdown
> restrictions (e.g. loading unsigned modules) should remain in effect.
>
> Regression Potential: Anyone using /proc/sysrq-trigger to disable
> lockdown will no longer be able to do so. Implementation bugs could
> prevent use of the sysrq from the keyboard from disabling lockdown, but
> this has been confrimed to still work with the fix in place.
>
> Note that xenial uses an older implementation of these patches which
> does not have any sysrq mechanism for lifting lockdown, and thus it is
> not affected.
>
> Thanks,
> Seth
>

Acked-by: Connor Kuehl <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED: [PATCH 0/2][SRU][B/D/E] Don't allow lifting of lockdown via /proc/sysrq-trigger

Khaled Elmously
In reply to this post by Seth Forshee
On 2019-11-05 14:35:03 , Seth Forshee wrote:

> BugLink: https://bugs.launchpad.net/bugs/1851380
>
> SRU Justification
>
> Impact: The kernel lockdown support adds a sysrq to allow a physically
> present user to disable lockdown from the keyboard. A bug in the
> implementation makes it possible to also lift lockdown by writing to
> /proc/sysrq-trigger.
>
> Fix: Correct the logic to disallow disabling lockdown via
> /proc/sysrq-trigger.
>
> Test Case: Write "x" to /proc/sysrq-trigger. When working properly there
> should be no messages in dmesg about lifting lockdown, and lockdown
> restrictions (e.g. loading unsigned modules) should remain in effect.
>
> Regression Potential: Anyone using /proc/sysrq-trigger to disable
> lockdown will no longer be able to do so. Implementation bugs could
> prevent use of the sysrq from the keyboard from disabling lockdown, but
> this has been confrimed to still work with the fix in place.
>
> Note that xenial uses an older implementation of these patches which
> does not have any sysrq mechanism for lifting lockdown, and thus it is
> not affected.
>
> Thanks,
> Seth
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team