* Re: [PATCH v3 1/1] NAX LSM: Add initial support
From: Randy Dunlap @ 2021-08-19 22:29 UTC (permalink / raw)
To: Igor Zhbanov, linux-integrity, linux-security-module, Mimi Zohar,
THOBY Simon, linux-kernel
In-Reply-To: <3f7db609-6393-163f-287f-2211ed6239a5@gmail.com>
Hi--
On 8/19/21 3:13 PM, Igor Zhbanov wrote:
> diff --git a/security/nax/Kconfig b/security/nax/Kconfig
> new file mode 100644
> index 000000000000..f0777cc38e17
> --- /dev/null
> +++ b/security/nax/Kconfig
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +config SECURITY_NAX
> + bool "NAX support"
> + depends on SECURITY
> + default n
'default n' is the default value and hence it is redundant.
We usually omit it.
> + help
> + This selects NAX (No Anonymous Execution), which extends DAC
> + support with additional system-wide security settings beyond
> + regular Linux discretionary access controls. Currently, the only
> + available behavior is restricting the execution of anonymous and
> + modified pages.
> +
> + The module can restrict either privileged or all processes,
> + depending on the settings. It is possible to configure action,
> + performed when the violation is detected (log, log + block,
> + log + kill).
> +
> + Further information can be found in
> + Documentation/admin-guide/LSM/NAX.rst.
> +
> + If you are unsure how to answer this question, answer N.
> +
> +choice
> + prompt "NAX violation action mode"
> + default SECURITY_NAX_MODE_LOG
> + depends on SECURITY_NAX
> + help
> + Select the NAX violation action mode.
> +
> + In the default permissive mode the violations are only logged
> + (if logging is not suppressed). In the enforcing mode the violations
> + are prohibited. And in the kill mode the process is terminated.
> +
> + The value can be overridden at boot time with the kernel command-line
> + parameter "nax_mode=" (0, 1, 2) or "kernel.nax.mode=" (0, 1, 2)
> + sysctl parameter (if the settings are not locked).
> +
> + config SECURITY_NAX_MODE_LOG
> + bool "Permissive mode"
> + help
> + In this mode violations are only logged (if logging is not
> + suppressed by the "kernel.nax.quiet" parameter). The
> + violating system call will not be prohibited.
> + config SECURITY_NAX_MODE_ENFORCING
> + bool "Enforcing mode"
> + help
> + In this mode violations are prohibited and logged (if
> + logging is not suppressed by the "kernel.nax.quiet"
> + parameter). The violating system call will return -EACCES
> + error.
> + config SECURITY_NAX_MODE_KILL
> + bool "Kill mode"
> + help
> + In this mode the violating process is terminated on the
> + first violation system call. The violation event is logged
> + (if logging is not suppressed by the "kernel.nax.quiet"
> + parameter).
> +endchoice
> +
> +config SECURITY_NAX_MODE
> + int
> + depends on SECURITY_NAX
> + default 0 if SECURITY_NAX_MODE_LOG
> + default 1 if SECURITY_NAX_MODE_ENFORCING
> + default 2 if SECURITY_NAX_MODE_KILL
> +
> +config SECURITY_NAX_CHECK_ALL
> + bool "Check all processes"
> + depends on SECURITY_NAX
> + help
> + If selected, NAX will check all processes. If not selected, NAX
> + will check only privileged processes (which is determined either
> + by having zero uid, euid, suid or fsuid; or by possessing
> + capabilities outside of allowed set).
> +
> + The value can also be overridden at boot time with the kernel
> + command-line parameter "nax_check_all=" (0, 1) or
> + "kernel.nax_check_all=" (0, 1) sysctl parameter (if the settings
kernel.nax.check_all ?
> + are not locked).
> +
> +config SECURITY_NAX_ALLOWED_CAPS
> + hex "Process capabilities ignored by NAX"
> + default 0x0
> + range 0x0 0xffffffffffff
Indent above line with tab + 2 spaces instead of all spaces.
> + depends on SECURITY_NAX
> + help
> + Hexadecimal number representing the set of capabilities
> + a non-root process can possess without being considered
> + "privileged" by NAX LSM.
> +
> + The value can be overridden at boot time with the command-line
> + parameter "nax_allowed_caps=" or "kernel.nax.allowed_caps=" sysctl
> + parameter (if the settings are not locked).
> +
> +config SECURITY_NAX_QUIET
> + bool "Silence NAX messages"
> + depends on SECURITY_NAX
> + help
> + If selected, NAX will not print violations.
> +
> + The value can be overridden at boot with the command-line
> + parameter "nax_quiet=" (0, 1) or "kernel.nax_quiet=" (0, 1) sysctl
kernel.nax.quiet
> + parameter (if the settings are not locked).
> +
> +config SECURITY_NAX_LOCKED
> + bool "Lock NAX settings"
> + depends on SECURITY_NAX
> + help
> + Pevent any update to the settings of the NAX LSM. This applies to
Prevent
> + both sysctl writes and the kernel command line.
> +
> + If not selected, it can be enabled at boot time with the kernel
> + command-line parameter "nax_locked=1" or "kernel.nax_locked=1"
kernel.nax.locked
> + sysctl parameter (if the settings are not locked).
--
~Randy
^ permalink raw reply
* [PATCH v3 1/1] NAX LSM: Add initial support
From: Igor Zhbanov @ 2021-08-19 22:13 UTC (permalink / raw)
To: linux-integrity, linux-security-module, Mimi Zohar, THOBY Simon,
linux-kernel
In-Reply-To: <adc0e031-f02d-775c-1148-e808013c1b97@gmail.com>
Add initial support for NAX (No Anonymous Execution), which is a Linux
Security Module that extends DAC by making impossible to make anonymous
and modified pages executable for privileged processes.
Intercepts anonymous executable pages created with mmap() and mprotect()
system calls.
Log violations (in non-quiet mode) and block the action or kill the
offending process, depending on the enabled settings.
See Documentation/admin-guide/LSM/NAX.rst.
Signed-off-by: Igor Zhbanov <izh1979@gmail.com>
---
Documentation/admin-guide/LSM/NAX.rst | 72 +++
Documentation/admin-guide/LSM/index.rst | 1 +
.../admin-guide/kernel-parameters.rst | 1 +
.../admin-guide/kernel-parameters.txt | 32 ++
security/Kconfig | 11 +-
security/Makefile | 2 +
security/nax/Kconfig | 114 +++++
security/nax/Makefile | 4 +
security/nax/nax-lsm.c | 472 ++++++++++++++++++
9 files changed, 704 insertions(+), 5 deletions(-)
create mode 100644 Documentation/admin-guide/LSM/NAX.rst
create mode 100644 security/nax/Kconfig
create mode 100644 security/nax/Makefile
create mode 100644 security/nax/nax-lsm.c
diff --git a/Documentation/admin-guide/LSM/NAX.rst b/Documentation/admin-guide/LSM/NAX.rst
new file mode 100644
index 000000000000..da54b3be4cda
--- /dev/null
+++ b/Documentation/admin-guide/LSM/NAX.rst
@@ -0,0 +1,72 @@
+=======
+NAX LSM
+=======
+
+:Author: Igor Zhbanov
+
+NAX (No Anonymous Execution) is a Linux Security Module that extends DAC
+by making impossible to make anonymous and modified pages executable for
+processes. The module intercepts anonymous executable pages created with
+mmap() and mprotect() system calls.
+
+To select it at boot time, add ``nax`` to ``security`` kernel command-line
+parameter.
+
+The following sysctl parameters are available:
+
+* ``kernel.nax.check_all``:
+ - 0: Check all processes.
+ - 1: Check only privileged processes. The privileged process is a process
+ for which any of the following is true:
+ - ``uid == 0``
+ - ``euid == 0``
+ - ``suid == 0``
+ - ``cap_effective`` has any capability except for the ones allowed
+ in ``kernel.nax.allowed_caps``
+ - ``cap_permitted`` has any capability except for the ones allowed
+ in ``kernel.nax.allowed_caps``
+
+ Checking of uid/euid/suid is important because a process may call seteuid(0)
+ to gain privileges (if SECURE_NO_SETUID_FIXUP secure bit is not set).
+
+* ``kernel.nax.allowed_caps``:
+
+ Hexadecimal number representing the set of capabilities a non-root
+ process can possess without being considered "privileged" by NAX LSM.
+
+ For the meaning of the capabilities bits and their value, please check
+ ``include/uapi/linux/capability.h`` and ``capabilities(7)`` manual page.
+
+ For example, ``CAP_SYS_PTRACE`` has a number 19. Therefore, to add it to
+ allowed capabilities list, we need to set 19'th bit (2^19 or 1 << 19)
+ or 80000 in hexadecimal form. Capabilities can be bitwise ORed.
+
+* ``kernel.nax.mode``:
+
+ - 0: Only log errors (when enabled by ``kernel.nax.quiet``) (default mode)
+ - 1: Forbid unsafe pages mappings (and log when enabled)
+ - 2: Kill the violating process (and log when enabled)
+
+* ``kernel.nax.quiet``:
+
+ - 0: Log violations (default)
+ - 1: Be quiet
+
+* ``kernel.nax.locked``:
+
+ - 0: Changing of the module's sysctl parameters is allowed
+ - 1: Further changing of the module's sysctl parameters is forbidden
+
+ Setting this parameter to ``1`` after initial setup during the system boot
+ will prevent the module disabling at the later time.
+
+There are matching kernel command-line parameters (with the same values):
+
+- ``nax_allowed_caps``
+- ``nax_check_all``
+- ``nax_mode``
+- ``nax_quiet``
+- ``nax_locked``
+
+The ``nax_locked`` command-line parameter must be specified last to avoid
+premature setting locking.
diff --git a/Documentation/admin-guide/LSM/index.rst b/Documentation/admin-guide/LSM/index.rst
index a6ba95fbaa9f..e9df7fc9a461 100644
--- a/Documentation/admin-guide/LSM/index.rst
+++ b/Documentation/admin-guide/LSM/index.rst
@@ -42,6 +42,7 @@ subdirectories.
apparmor
LoadPin
+ NAX
SELinux
Smack
tomoyo
diff --git a/Documentation/admin-guide/kernel-parameters.rst b/Documentation/admin-guide/kernel-parameters.rst
index 01ba293a2d70..f4e91dc729f0 100644
--- a/Documentation/admin-guide/kernel-parameters.rst
+++ b/Documentation/admin-guide/kernel-parameters.rst
@@ -136,6 +136,7 @@ parameter is applicable::
MOUSE Appropriate mouse support is enabled.
MSI Message Signaled Interrupts (PCI).
MTD MTD (Memory Technology Device) support is enabled.
+ NAX NAX support is enabled.
NET Appropriate network support is enabled.
NUMA NUMA support is enabled.
NFS Appropriate NFS support is enabled.
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index bdb22006f713..10ed55e28d49 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3100,6 +3100,38 @@
n2= [NET] SDL Inc. RISCom/N2 synchronous serial card
+ nax_allowed_caps= [NAX] Hexadecimal number representing the set of
+ capabilities a non-root process can possess without
+ being considered "privileged" by NAX LSM.
+
+ For the meaning of the capabilities bits and their
+ value, please check include/uapi/linux/capability.h
+ and capabilities(7) manual page.
+
+ For example, `CAP_SYS_PTRACE` has a number 19.
+ Therefore, to add it to allowed capabilities list,
+ we need to set 19'th bit (2^19 or 1 << 19) or 80000
+ in hexadecimal form. Capabilities can be bitwise ORed.
+
+ nax_check_all= [NAX] NAX LSM processes checking mode:
+ 0 - Check only privileged processes (default).
+ 1 - Check all processes.
+
+ nax_locked= [NAX] NAX LSM settings' locking mode:
+ 0 - Changing NAX sysctl parameters is allowed.
+ 1 - Changing NAX sysctl parameters is forbidden until
+ reboot.
+
+ nax_mode= [NAX] NAX LSM violation reaction mode:
+ 0 - Only log errors (when not in quiet mode; default).
+ 1 - Forbid unsafe pages mappings (and log when
+ enabled).
+ 2 - Kill the violating process (and log when enabled).
+
+ nax_quiet= [NAX] NAX LSM log verbosity:
+ 0 - Log messages to syslog.
+ 1 - Be quiet.
+
netdev= [NET] Network devices parameters
Format: <irq>,<io>,<mem_start>,<mem_end>,<name>
Note that mem_start is often overloaded to mean
diff --git a/security/Kconfig b/security/Kconfig
index 0ced7fd33e4d..771419647ae1 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,6 +239,7 @@ source "security/yama/Kconfig"
source "security/safesetid/Kconfig"
source "security/lockdown/Kconfig"
source "security/landlock/Kconfig"
+source "security/nax/Kconfig"
source "security/integrity/Kconfig"
@@ -278,11 +279,11 @@ endchoice
config LSM
string "Ordered list of enabled LSMs"
- default "landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
- default "landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
- default "landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
- default "landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
- default "landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
+ default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
+ default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
+ default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
+ default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
+ default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
diff --git a/security/Makefile b/security/Makefile
index 47e432900e24..5c261bbf4659 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -14,6 +14,7 @@ subdir-$(CONFIG_SECURITY_SAFESETID) += safesetid
subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
subdir-$(CONFIG_BPF_LSM) += bpf
subdir-$(CONFIG_SECURITY_LANDLOCK) += landlock
+subdir-$(CONFIG_SECURITY_NAX) += nax
# always enable default capabilities
obj-y += commoncap.o
@@ -34,6 +35,7 @@ obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/
obj-$(CONFIG_CGROUPS) += device_cgroup.o
obj-$(CONFIG_BPF_LSM) += bpf/
obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/
+obj-$(CONFIG_SECURITY_NAX) += nax/
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
diff --git a/security/nax/Kconfig b/security/nax/Kconfig
new file mode 100644
index 000000000000..f0777cc38e17
--- /dev/null
+++ b/security/nax/Kconfig
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config SECURITY_NAX
+ bool "NAX support"
+ depends on SECURITY
+ default n
+ help
+ This selects NAX (No Anonymous Execution), which extends DAC
+ support with additional system-wide security settings beyond
+ regular Linux discretionary access controls. Currently, the only
+ available behavior is restricting the execution of anonymous and
+ modified pages.
+
+ The module can restrict either privileged or all processes,
+ depending on the settings. It is possible to configure action,
+ performed when the violation is detected (log, log + block,
+ log + kill).
+
+ Further information can be found in
+ Documentation/admin-guide/LSM/NAX.rst.
+
+ If you are unsure how to answer this question, answer N.
+
+choice
+ prompt "NAX violation action mode"
+ default SECURITY_NAX_MODE_LOG
+ depends on SECURITY_NAX
+ help
+ Select the NAX violation action mode.
+
+ In the default permissive mode the violations are only logged
+ (if logging is not suppressed). In the enforcing mode the violations
+ are prohibited. And in the kill mode the process is terminated.
+
+ The value can be overridden at boot time with the kernel command-line
+ parameter "nax_mode=" (0, 1, 2) or "kernel.nax.mode=" (0, 1, 2)
+ sysctl parameter (if the settings are not locked).
+
+ config SECURITY_NAX_MODE_LOG
+ bool "Permissive mode"
+ help
+ In this mode violations are only logged (if logging is not
+ suppressed by the "kernel.nax.quiet" parameter). The
+ violating system call will not be prohibited.
+ config SECURITY_NAX_MODE_ENFORCING
+ bool "Enforcing mode"
+ help
+ In this mode violations are prohibited and logged (if
+ logging is not suppressed by the "kernel.nax.quiet"
+ parameter). The violating system call will return -EACCES
+ error.
+ config SECURITY_NAX_MODE_KILL
+ bool "Kill mode"
+ help
+ In this mode the violating process is terminated on the
+ first violation system call. The violation event is logged
+ (if logging is not suppressed by the "kernel.nax.quiet"
+ parameter).
+endchoice
+
+config SECURITY_NAX_MODE
+ int
+ depends on SECURITY_NAX
+ default 0 if SECURITY_NAX_MODE_LOG
+ default 1 if SECURITY_NAX_MODE_ENFORCING
+ default 2 if SECURITY_NAX_MODE_KILL
+
+config SECURITY_NAX_CHECK_ALL
+ bool "Check all processes"
+ depends on SECURITY_NAX
+ help
+ If selected, NAX will check all processes. If not selected, NAX
+ will check only privileged processes (which is determined either
+ by having zero uid, euid, suid or fsuid; or by possessing
+ capabilities outside of allowed set).
+
+ The value can also be overridden at boot time with the kernel
+ command-line parameter "nax_check_all=" (0, 1) or
+ "kernel.nax_check_all=" (0, 1) sysctl parameter (if the settings
+ are not locked).
+
+config SECURITY_NAX_ALLOWED_CAPS
+ hex "Process capabilities ignored by NAX"
+ default 0x0
+ range 0x0 0xffffffffffff
+ depends on SECURITY_NAX
+ help
+ Hexadecimal number representing the set of capabilities
+ a non-root process can possess without being considered
+ "privileged" by NAX LSM.
+
+ The value can be overridden at boot time with the command-line
+ parameter "nax_allowed_caps=" or "kernel.nax.allowed_caps=" sysctl
+ parameter (if the settings are not locked).
+
+config SECURITY_NAX_QUIET
+ bool "Silence NAX messages"
+ depends on SECURITY_NAX
+ help
+ If selected, NAX will not print violations.
+
+ The value can be overridden at boot with the command-line
+ parameter "nax_quiet=" (0, 1) or "kernel.nax_quiet=" (0, 1) sysctl
+ parameter (if the settings are not locked).
+
+config SECURITY_NAX_LOCKED
+ bool "Lock NAX settings"
+ depends on SECURITY_NAX
+ help
+ Pevent any update to the settings of the NAX LSM. This applies to
+ both sysctl writes and the kernel command line.
+
+ If not selected, it can be enabled at boot time with the kernel
+ command-line parameter "nax_locked=1" or "kernel.nax_locked=1"
+ sysctl parameter (if the settings are not locked).
diff --git a/security/nax/Makefile b/security/nax/Makefile
new file mode 100644
index 000000000000..9c3372210c77
--- /dev/null
+++ b/security/nax/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_SECURITY_NAX) := nax.o
+
+nax-y := nax-lsm.o
diff --git a/security/nax/nax-lsm.c b/security/nax/nax-lsm.c
new file mode 100644
index 000000000000..5ff3ba12079d
--- /dev/null
+++ b/security/nax/nax-lsm.c
@@ -0,0 +1,472 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2016-2021 Open Mobile Platform LLC.
+ *
+ * Written by: Igor Zhbanov <i.zhbanov@omp.ru, izh1979@gmail.com>
+ *
+ * NAX (No Anonymous Execution) Linux Security Module
+ * This module prevents execution of the code in anonymous or modified pages.
+ * For more details, see Documentation/admin-guide/LSM/NAX.rst and
+ * Documentation/admin-guide/kernel-parameters.rst
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2, as
+ * published by the Free Software Foundation.
+ */
+
+#define pr_fmt(fmt) "NAX: " fmt
+
+#include <linux/capability.h>
+#include <linux/cred.h>
+#include <linux/ctype.h>
+#include <linux/lsm_hooks.h>
+#include <linux/mman.h>
+#include <linux/rcupdate.h>
+#include <linux/sched.h>
+#include <linux/securebits.h>
+#include <linux/security.h>
+#include <linux/spinlock.h>
+#include <linux/sysctl.h>
+#include <linux/uidgid.h>
+
+#define NAX_MODE_PERMISSIVE 0 /* Log only */
+#define NAX_MODE_ENFORCING 1 /* Enforce and log */
+#define NAX_MODE_KILL 2 /* Kill process and log */
+
+static int mode = CONFIG_SECURITY_NAX_MODE,
+ quiet = IS_ENABLED(CONFIG_SECURITY_NAX_QUIET),
+ locked = IS_ENABLED(CONFIG_SECURITY_NAX_LOCKED),
+ check_all = IS_ENABLED(CONFIG_SECURITY_NAX_CHECK_ALL);
+
+#define ALLOWED_CAPS_HEX_LEN (_KERNEL_CAPABILITY_U32S * 8)
+
+static char allowed_caps_hex[ALLOWED_CAPS_HEX_LEN + 1];
+static kernel_cap_t __rcu *allowed_caps;
+DEFINE_SPINLOCK(allowed_caps_mutex);
+
+static bool
+is_interesting_process(void)
+{
+ bool ret = false;
+ const struct cred *cred;
+ kuid_t root_uid;
+ kernel_cap_t *caps;
+
+ if (check_all)
+ return true;
+
+ cred = current_cred();
+ root_uid = make_kuid(cred->user_ns, 0);
+
+ rcu_read_lock();
+ caps = rcu_dereference(allowed_caps);
+ /*
+ * We count a process as interesting if it any of its uid/euid/suid
+ * is zero (because it may call seteuid(0) to gain privileges) or
+ * it has any not allowed capability (even in a user namespace)
+ */
+ if ((!issecure(SECURE_NO_SETUID_FIXUP) &&
+ (uid_eq(cred->uid, root_uid) ||
+ uid_eq(cred->euid, root_uid) ||
+ uid_eq(cred->suid, root_uid))) ||
+ (!cap_issubset(cred->cap_effective, *caps)) ||
+ (!cap_issubset(cred->cap_permitted, *caps)))
+ ret = true;
+
+ rcu_read_unlock();
+ return ret;
+}
+
+static void
+log_warn(const char *reason)
+{
+ if (quiet)
+ return;
+
+ pr_warn_ratelimited("%s: pid=%d, uid=%u, comm=\"%s\"\n",
+ reason, current->pid,
+ from_kuid(&init_user_ns, current_cred()->uid),
+ current->comm);
+}
+
+static void
+kill_current_task(void)
+{
+ pr_warn("Killing pid=%d, uid=%u, comm=\"%s\"\n",
+ current->pid, from_kuid(&init_user_ns, current_cred()->uid),
+ current->comm);
+ force_sig(SIGKILL);
+}
+
+static int
+nax_mmap_file(struct file *file, unsigned long reqprot,
+ unsigned long prot, unsigned long flags)
+{
+ int ret = 0;
+
+ if (mode == NAX_MODE_PERMISSIVE && quiet)
+ return 0; /* Skip further checks in this case */
+
+ if (!(prot & PROT_EXEC)) /* Not executable memory */
+ return 0;
+
+ if (!is_interesting_process())
+ return 0; /* Not interesting processes can do anything */
+
+ if (!file) { /* Anonymous executable memory */
+ log_warn("MMAP_ANON_EXEC");
+ ret = -EACCES;
+ } else if (prot & PROT_WRITE) { /* Mapping file RWX */
+ log_warn("MMAP_FILE_WRITE_EXEC");
+ ret = -EACCES;
+ }
+
+ if (ret && mode == NAX_MODE_KILL)
+ kill_current_task();
+
+ return (mode != NAX_MODE_PERMISSIVE) ? ret : 0;
+}
+
+static int
+nax_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+ unsigned long prot)
+{
+ int ret = 0;
+
+ if (mode == NAX_MODE_PERMISSIVE && quiet)
+ return 0; /* Skip further checks in this case */
+
+ if (!(prot & PROT_EXEC)) /* Not executable memory */
+ return 0;
+
+ if (!is_interesting_process())
+ return 0; /* Not interesting processes can do anything */
+
+ if (!(vma->vm_flags & VM_EXEC)) {
+ if (vma->vm_start >= vma->vm_mm->start_brk &&
+ vma->vm_end <= vma->vm_mm->brk) {
+ log_warn("MPROTECT_EXEC_HEAP");
+ ret = -EACCES;
+ } else if (!vma->vm_file &&
+ ((vma->vm_start <= vma->vm_mm->start_stack &&
+ vma->vm_end >= vma->vm_mm->start_stack) ||
+ vma_is_stack_for_current(vma))) {
+ log_warn("MPROTECT_EXEC_STACK");
+ ret = -EACCES;
+ } else if (vma->vm_file && vma->anon_vma) {
+ /*
+ * We are making executable a file mapping that has
+ * had some COW done. Since pages might have been
+ * written, check ability to execute the possibly
+ * modified content. This typically should only
+ * occur for text relocations.
+ */
+ log_warn("MPROTECT_EXEC_MODIFIED");
+ ret = -EACCES;
+ }
+ }
+
+ if (!ret) {
+ if (!vma->vm_file) { /* Anonymous executable memory */
+ log_warn("MPROTECT_ANON_EXEC");
+ ret = -EACCES;
+ } else if (prot & PROT_WRITE) { /* Remapping file as RWX */
+ log_warn("MPROTECT_FILE_WRITE_EXEC");
+ ret = -EACCES;
+ }
+ }
+
+ if (ret && mode == NAX_MODE_KILL)
+ kill_current_task();
+
+ return (mode != NAX_MODE_PERMISSIVE) ? ret : 0;
+}
+
+static struct security_hook_list nax_hooks[] __lsm_ro_after_init = {
+ LSM_HOOK_INIT(mmap_file, nax_mmap_file),
+ LSM_HOOK_INIT(file_mprotect, nax_file_mprotect),
+};
+
+static void
+update_allowed_caps(kernel_cap_t *caps)
+{
+ kernel_cap_t *old_caps;
+
+ *caps = cap_intersect(*caps, CAP_FULL_SET); /* Drop unsupported */
+ spin_lock(&allowed_caps_mutex);
+ old_caps = rcu_dereference_protected(allowed_caps,
+ lockdep_is_held(&allowed_caps_mutex));
+ rcu_assign_pointer(allowed_caps, caps);
+ spin_unlock(&allowed_caps_mutex);
+ synchronize_rcu();
+ kfree(old_caps);
+}
+
+static int
+set_default_allowed_caps(void)
+{
+ size_t i;
+ kernel_cap_t *caps;
+
+ caps = kmalloc(sizeof(*caps), GFP_KERNEL);
+ if (!caps)
+ return -ENOMEM;
+
+ CAP_FOR_EACH_U32(i)
+ caps->cap[i] = (CONFIG_SECURITY_NAX_ALLOWED_CAPS >> (i * 8)) &
+ 0xff;
+
+ update_allowed_caps(caps);
+ return 0;
+}
+
+static int
+parse_and_set_caps(char *str)
+{
+ size_t len, i;
+ kernel_cap_t *caps;
+
+ /* len is guaranteed not to exceed ALLOWED_CAPS_HEX_LEN */
+ len = strlen(str);
+ for (i = 0; i < len; i++)
+ if (!isxdigit(str[i]))
+ return -EINVAL;
+
+ caps = kmalloc(sizeof(*caps), GFP_KERNEL);
+ if (!caps)
+ return -ENOMEM;
+
+ CAP_FOR_EACH_U32(i) {
+ unsigned long l;
+
+ if (kstrtoul(str + (len >= 8 ? len - 8 : 0), 16, &l))
+ return -EINVAL;
+
+ caps->cap[i] = l;
+ if (len < 8)
+ break;
+
+ len -= 8;
+ str[len] = '\0';
+ }
+
+ update_allowed_caps(caps);
+ return 0;
+}
+
+#ifdef CONFIG_SYSCTL
+
+static int
+nax_dointvec_minmax(struct ctl_table *table, int write,
+ void *buffer, size_t *lenp, loff_t *ppos)
+{
+ if (write && (!capable(CAP_SYS_ADMIN) || locked))
+ return -EPERM;
+
+ return proc_dointvec_minmax(table, write, buffer, lenp, ppos);
+}
+
+static int
+nax_dostring(struct ctl_table *table, int write, void *buffer,
+ size_t *lenp, loff_t *ppos)
+{
+ int ret;
+
+ if (write) { /* A user is setting the allowed capabilities */
+ int error;
+ char *buf = (char *)buffer;
+ size_t len = *lenp;
+
+ if (!capable(CAP_SYS_ADMIN) || locked)
+ return -EPERM;
+
+ /* Do not allow trailing garbage or excessive length */
+ if (len == ALLOWED_CAPS_HEX_LEN + 1) {
+ if (buf[--len] != '\n')
+ return -EINVAL;
+ } else if (len > ALLOWED_CAPS_HEX_LEN || len <= 0) {
+ return -EINVAL;
+ }
+
+ error = proc_dostring(table, write, buffer, lenp, ppos);
+ if (error)
+ return error;
+
+ ret = parse_and_set_caps(allowed_caps_hex);
+ } else { /* A user is getting the allowed capabilities */
+ unsigned int i;
+ kernel_cap_t *caps;
+
+ rcu_read_lock();
+ caps = rcu_dereference(allowed_caps);
+ CAP_FOR_EACH_U32(i)
+ snprintf(allowed_caps_hex + i * 8, 9, "%08x",
+ caps->cap[CAP_LAST_U32 - i]);
+
+ rcu_read_unlock();
+ ret = proc_dostring(table, write, buffer, lenp, ppos);
+ }
+
+ return ret;
+}
+
+struct ctl_path nax_sysctl_path[] = {
+ { .procname = "kernel" },
+ { .procname = "nax" },
+ { }
+};
+
+static int max_mode = NAX_MODE_KILL;
+
+static struct ctl_table nax_sysctl_table[] = {
+ {
+ .procname = "allowed_caps",
+ .data = allowed_caps_hex,
+ .maxlen = ALLOWED_CAPS_HEX_LEN + 1,
+ .mode = 0644,
+ .proc_handler = nax_dostring,
+ }, {
+ .procname = "check_all",
+ .data = &check_all,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = nax_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ }, {
+ .procname = "locked",
+ .data = &locked,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = nax_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ }, {
+ .procname = "mode",
+ .data = &mode,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = nax_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = &max_mode,
+ }, {
+ .procname = "quiet",
+ .data = &quiet,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = nax_dointvec_minmax,
+ .extra1 = SYSCTL_ZERO,
+ .extra2 = SYSCTL_ONE,
+ },
+ { }
+};
+
+static void __init
+nax_init_sysctl(void)
+{
+ if (!register_sysctl_paths(nax_sysctl_path, nax_sysctl_table))
+ panic("NAX: sysctl registration failed.\n");
+}
+
+#else /* !CONFIG_SYSCTL */
+
+static inline void
+nax_init_sysctl(void)
+{
+
+}
+
+#endif /* !CONFIG_SYSCTL */
+
+static int __init setup_allowed_caps(char *str)
+{
+ if (locked)
+ return 1;
+
+ /* Do not allow trailing garbage or excessive length */
+ if (strlen(str) > ALLOWED_CAPS_HEX_LEN) {
+ pr_err("Invalid 'nax_allowed_caps' parameter value (%s)\n",
+ str);
+ return 1;
+ }
+
+ strscpy(allowed_caps_hex, str, sizeof(allowed_caps_hex));
+ if (parse_and_set_caps(allowed_caps_hex))
+ pr_err("Invalid 'nax_allowed_caps' parameter value (%s)\n",
+ str);
+
+ return 1;
+}
+__setup("nax_allowed_caps=", setup_allowed_caps);
+
+static int __init setup_check_all(char *str)
+{
+ unsigned long val;
+
+ if (!locked && !kstrtoul(str, 0, &val))
+ check_all = val ? 1 : 0;
+
+ return 1;
+}
+__setup("nax_quiet=", setup_check_all);
+
+static int __init setup_locked(char *str)
+{
+ unsigned long val;
+
+ if (!locked && !kstrtoul(str, 0, &val))
+ locked = val ? 1 : 0;
+
+ return 1;
+}
+__setup("nax_locked=", setup_locked);
+
+static int __init setup_mode(char *str)
+{
+ unsigned long val;
+
+ if (!locked && !kstrtoul(str, 0, &val)) {
+ if (val > max_mode) {
+ pr_err("Invalid 'nax_mode' parameter value (%s)\n",
+ str);
+ val = max_mode;
+ }
+
+ mode = val;
+ }
+
+ return 1;
+}
+__setup("nax_mode=", setup_mode);
+
+static int __init setup_quiet(char *str)
+{
+ unsigned long val;
+
+ if (!locked && !kstrtoul(str, 0, &val))
+ quiet = val ? 1 : 0;
+
+ return 1;
+}
+__setup("nax_quiet=", setup_quiet);
+
+static __init int
+nax_init(void)
+{
+ int rc;
+
+ pr_info("Starting.\n");
+ rc = set_default_allowed_caps();
+ if (rc < 0)
+ return rc;
+
+ security_add_hooks(nax_hooks, ARRAY_SIZE(nax_hooks), "nax");
+ nax_init_sysctl();
+
+ return 0;
+}
+
+DEFINE_LSM(nax) = {
+ .name = "nax",
+ .init = nax_init,
+};
--
2.26.2
^ permalink raw reply related
* [PATCH v3 0/1] NAX (No Anonymous Execution) LSM
From: Igor Zhbanov @ 2021-08-19 22:12 UTC (permalink / raw)
To: linux-integrity, linux-security-module, Mimi Zohar, THOBY Simon,
linux-kernel
[Overview]
Fileless malware attacks are becoming more and more popular, and even
ready-to-use frameworks are available [1], [2], [3]. They are based on
running of the malware code from anonymous executable memory pages (which
are not backed by an executable file or a library on a filesystem.) This
allows effectively hiding malware presence in a system, making filesystem
integrity checking tools unable to detect the intrusion.
Typically, the malware first needs to intercept the execution flow (e.g.,
by the means of ROP-based exploit). Then it needs to download the main
part (in the form of normal executable or library) from its server,
because it is hard to implement the entire exploit in ROP-based form.
There are a number of security mechanisms that can ensure the integrity
of the file-system, but we need to ensure the integrity of the code in
memory too, to be sure, that only authorized code is running in the
system.
The proposed LSM is preventing the creation of anonymous executable pages
for the processes. The LSM intercepts mmap() and mprotect() system calls
and handles it similarly to SELinux handlers.
The module allows to block the violating system call or to kill the
violating process, depending on the settings, along with rate-limited
logging.
Currently, the module restricts ether all processes or only the
privileged processes, depending on the settings. The privileged process
is a process for which any of the following is true:
+ uid == 0 && !issecure(SECURE_NOROOT)
+ euid == 0 && !issecure(SECURE_NOROOT)
+ suid == 0 && !issecure(SECURE_NOROOT)
+ cap_effective has any capability except of kernel.nax.allowed_caps
+ cap_permitted has any capability except of kernel.nax.allowed_caps
Checking of uid/euid/suid is important because a process may call
seteuid(0) to gain privileges (if SECURE_NO_SETUID_FIXUP secure bit
is not set).
The sysctl parameter kernel.nax.allowed_caps allows to define safe
capabilities set for the privileged processes.
[JIT]
Because of blocked anonymous code execution, JIT-compiled code, some
interpreters (which are using JIT) and libffi-based projects can be
broken.
Our observation shows that such processes are typically running by a
user, so they will not be privileged, so they will be allowed to use
anonymous executable pages.
But for small embedded set-ups it could be possible to get rid of such
processes at all, so the module could be enabled without further
restrictions to protect both privileged and non-privileged processes.
In addition, libffi can be modified not to use anonymous executable
pages.
[Similar implementations]
Although SELinux could be used to enable similar functionality, this LSM
is simpler. It could be used in set-ups, where SELinux would be overkill.
There is also SARA LSM module, which solves similar task, but it is more
complex.
[Cooperation with other security mechanisms]
NAX LSM is more useful in conjunction with IMA. IMA would be responsible
for integrity checking of file-based executables and libraries, and
NAX LSM would be responsible for preventing of anonymous code execution.
Alternatively, NAX LSM can be used with read-only root file system,
protected by dm-verity/fs-verity.
[TODO]
- Implement xattrs support for marking privileged binaries on a per-file
basis.
- Store NAX attributes in the per-task LSM blob to implement special
launchers for the privileged processes, so all of the children processes
of such a launcher would be allowed to have anonymous executable pages
(but not to grandchildren).
[Links]
[1] https://blog.fbkcs.ru/elf-in-memory-execution/
[2] https://magisterquis.github.io/2018/03/31/in-memory-only-elf-execution.html
[3] https://www.prodefence.org/fireelf-fileless-linux-malware-framework/
[Credits]
Thanks to Mimi Zohar for consulting and to Simon Thoby for thorough review.
[Changelog]
V3
- Fix memory leak in allowed_caps assigning code.
- Protect allowed_caps updating with a spinlock.
- Fix Kconfig options description.
- Add example for allowed_caps value.
- Fix typo in documentation.
V2
- Fixed typo in Kconfig.
- Fixed "cap_effective" and "cap_permitted" parameters description in NAX.rst.
- Added "nax_allowed_caps" setup parameter. Factored out capabilities parsing
logic.
- Added parameter for checking all processes (not only privileged).
- Added Kconfig parameter for setting allowed capabilities.
- Updated nax_file_mprotect() to avoid calling of nax_mmap_file() to avoid
duplicated checks.
- Protect allowed_caps with RCU.
- Fixed all errors and most warning found by checkpatch.pl.
- Updated the module documentation. Added description of the boot parameters to
kernel-parameters.
- Updated commit message.
V1:
- Initial implementation.
Igor Zhbanov (1):
NAX LSM: Add initial support
Documentation/admin-guide/LSM/NAX.rst | 72 +++
Documentation/admin-guide/LSM/index.rst | 1 +
.../admin-guide/kernel-parameters.rst | 1 +
.../admin-guide/kernel-parameters.txt | 32 ++
security/Kconfig | 11 +-
security/Makefile | 2 +
security/nax/Kconfig | 114 +++++
security/nax/Makefile | 4 +
security/nax/nax-lsm.c | 472 ++++++++++++++++++
9 files changed, 704 insertions(+), 5 deletions(-)
create mode 100644 Documentation/admin-guide/LSM/NAX.rst
create mode 100644 security/nax/Kconfig
create mode 100644 security/nax/Makefile
create mode 100644 security/nax/nax-lsm.c
--
2.26.2
^ permalink raw reply
* Re: [PATCH] ima: fix infinite loop within "ima_match_policy" function.
From: Mimi Zohar @ 2021-08-19 19:31 UTC (permalink / raw)
To: THOBY Simon, liqiong
Cc: dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ed27351e0574f58ee59a3024554b8b0c7293515f.camel@linux.ibm.com>
On Thu, 2021-08-19 at 09:47 -0400, Mimi Zohar wrote:
> On Thu, 2021-08-19 at 12:58 +0000, THOBY Simon wrote:
> > Hi Liqiong,
> >
> > On 8/19/21 12:15 PM, liqiong wrote:
> > > When "ima_match_policy" is looping while "ima_update_policy" changs
> > > the variable "ima_rules", then "ima_match_policy" may can't exit loop,
> > > and kernel keeps printf "rcu_sched detected stall on CPU ...".
> > >
> > > It occurs at boot phase, systemd-services are being checked within
> > > "ima_match_policy,at the same time, the variable "ima_rules"
> > > is changed by a service.
> >
> > First off, thanks for finding and identifying this nasty bug.
>
> Once the initial builtin policy rules have been replaced by a custom
> policy, rules may only be appended by splicing the new rules with the
> existing rules. There should never be a problem reading the rules at
> that point. Does this problem occur before the builtin policy rules
> have been replaced with a custom policy?
Yes, the problem is limited to transitioning from the builtin policy to
the custom policy. Adding a new lock around rcu code seems counter
productive, especially since switching the policy rules happens once,
normally during early boot before access to real root. Please consider
Simon's suggestion or finding some other solution.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
From: Mimi Zohar @ 2021-08-19 17:32 UTC (permalink / raw)
To: Eric Snowberg, Jarkko Sakkinen, David Howells
Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
David S . Miller, James Morris, Serge E . Hallyn, keescook,
gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
linux-crypto, linux-security-module, James Bottomley, pjones,
konrad.wilk@oracle.com, Patrick Uiterwijk
In-Reply-To: <91B1FE51-C6FC-4ADF-B05A-B1E59E20132E@oracle.com>
On Thu, 2021-08-19 at 09:23 -0600, Eric Snowberg wrote:
> > On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> >> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> >>> Downstream Linux distros try to have a single signed kernel for each
> >>> architecture. Each end-user may use this kernel in entirely different
> >>> ways. Some downstream kernels have chosen to always trust platform keys
> >>> within the Linux trust boundary for kernel module signing. These
> >>> kernels have no way of using digital signature base IMA appraisal.
> >>>
> >>> This series introduces a new Linux kernel keyring containing the Machine
> >>> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
> >>
> >> I would name it as ".machine" because it is more "re-usable" name, e.g.
> >> could be used for similar things as MOK. ".mok" is a bad name because
> >> it binds directly to a single piece of user space software.
> >
> > Nayna previously said,
> > "I believe the underlying source from where CA keys are loaded might vary
> > based on the architecture (".mok" is UEFI specific.). The key part is
> > that this new keyring should contain only CA keys which can be later
> > used to vouch for user keys loaded onto IMA or secondary keyring at
> > runtime. It would be good to have a "ca" in the name, like .xxxx-ca,
> > where xxxx can be machine, owner, or system. I prefer .system-ca."
> >
> > The CA keys on the MOK db is simply the first root of trust being
> > defined, but other roots of trust are sure to follow. For this reason,
> > I agree naming the new keyring "mok" should be avoided.
>
> As I said previously, I’m open to renaming, I just would like to have an
> agreement on the new name before changing everything. The current proposed
> names I have heard are “.machine" and ".system-ca". Is there a preference
> the maintainers feel is appropriate? If so, please let me know and I’ll
> rename it. Thanks.
>
Jarkko, I think the emphasis should not be on "machine" from Machine
Owner Key (MOK), but on "owner". Whereas Nayna is focusing more on the
"_ca" aspect of the name. Perhaps consider naming it
"system_owner_ca" or something along those lines.
thanks,
Mimi
^ permalink raw reply
* [syzbot] UBSAN: array-index-out-of-bounds in ima_inode_setxattr
From: syzbot @ 2021-08-19 15:30 UTC (permalink / raw)
To: dmitry.kasatkin, jmorris, linux-integrity, linux-kernel,
linux-security-module, serge, syzkaller-bugs, zohar
Hello,
syzbot found the following issue on:
HEAD commit: 33e65b1f975c Add linux-next specific files for 20210819
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1546c341300000
kernel config: https://syzkaller.appspot.com/x/.config?x=3022de5bd1dbc8f5
dashboard link: https://syzkaller.appspot.com/bug?extid=e8bafe7b82c739eaf153
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15767d41300000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13582731300000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+e8bafe7b82c739eaf153@syzkaller.appspotmail.com
================================================================================
UBSAN: array-index-out-of-bounds in security/integrity/ima/ima_appraise.c:621:36
index 222 is out of range for type 'char *[20]'
CPU: 1 PID: 6550 Comm: syz-executor680 Not tainted 5.14.0-rc6-next-20210819-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xcd/0x134 lib/dump_stack.c:106
ubsan_epilogue+0xb/0x5a lib/ubsan.c:151
__ubsan_handle_out_of_bounds.cold+0x64/0x70 lib/ubsan.c:291
validate_hash_algo security/integrity/ima/ima_appraise.c:621 [inline]
ima_inode_setxattr+0x536/0x540 security/integrity/ima/ima_appraise.c:656
security_inode_setxattr+0x148/0x240 security/security.c:1355
__vfs_setxattr_locked+0xa7/0x260 fs/xattr.c:266
vfs_setxattr+0x14e/0x350 fs/xattr.c:301
setxattr+0x21b/0x2b0 fs/xattr.c:575
path_setxattr+0x19d/0x1d0 fs/xattr.c:595
__do_sys_lsetxattr fs/xattr.c:618 [inline]
__se_sys_lsetxattr fs/xattr.c:614 [inline]
__x64_sys_lsetxattr+0xbd/0x150 fs/xattr.c:614
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x39/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x44/0xae
RIP: 0033:0x43ee89
Code: 28 c3 e8 2a 14 00 00 66 2e 0f 1f 84 00 00 00 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 c0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007ffe96b06768 EFLAGS: 00000246 ORIG_RAX: 00000000000000bd
RAX: ffffffffffffffda RBX: 0000000000400488 RCX: 000000000043ee89
RDX: 0000000020000140 RSI: 00000000200000c0 RDI: 0000000020000000
RBP: 0000000000402e70 R08: 0000000000000000 R09: 0000000000000000
R10: 000000000000000a R11: 0000000000000246 R12: 0000000000402f00
R13: 0000000000000000 R14: 00000000004ac018 R15: 0000000000400488
================================================================================
----------------
Code disassembly (best guess):
0: 28 c3 sub %al,%bl
2: e8 2a 14 00 00 callq 0x1431
7: 66 2e 0f 1f 84 00 00 nopw %cs:0x0(%rax,%rax,1)
e: 00 00 00
11: 48 89 f8 mov %rdi,%rax
14: 48 89 f7 mov %rsi,%rdi
17: 48 89 d6 mov %rdx,%rsi
1a: 48 89 ca mov %rcx,%rdx
1d: 4d 89 c2 mov %r8,%r10
20: 4d 89 c8 mov %r9,%r8
23: 4c 8b 4c 24 08 mov 0x8(%rsp),%r9
28: 0f 05 syscall
2a: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax <-- trapping instruction
30: 73 01 jae 0x33
32: c3 retq
33: 48 c7 c1 c0 ff ff ff mov $0xffffffffffffffc0,%rcx
3a: f7 d8 neg %eax
3c: 64 89 01 mov %eax,%fs:(%rcx)
3f: 48 rex.W
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
From: Eric Snowberg @ 2021-08-19 15:23 UTC (permalink / raw)
To: Mimi Zohar, Jarkko Sakkinen, David Howells
Cc: keyrings, linux-integrity, David Woodhouse, Herbert Xu,
David S . Miller, James Morris, Serge E . Hallyn, keescook,
gregkh, torvalds, scott.branden, weiyongjun1, nayna, ebiggers,
ardb, Lakshmi Ramasubramanian, lszubowi, linux-kernel,
linux-crypto, linux-security-module, James Bottomley, pjones,
konrad.wilk@oracle.com, Patrick Uiterwijk
In-Reply-To: <f76fcf41728fbdd65f2b3464df0821f248b2cba0.camel@linux.ibm.com>
> On Aug 19, 2021, at 7:10 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
>> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
>>> Downstream Linux distros try to have a single signed kernel for each
>>> architecture. Each end-user may use this kernel in entirely different
>>> ways. Some downstream kernels have chosen to always trust platform keys
>>> within the Linux trust boundary for kernel module signing. These
>>> kernels have no way of using digital signature base IMA appraisal.
>>>
>>> This series introduces a new Linux kernel keyring containing the Machine
>>> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
>>
>> I would name it as ".machine" because it is more "re-usable" name, e.g.
>> could be used for similar things as MOK. ".mok" is a bad name because
>> it binds directly to a single piece of user space software.
>
> Nayna previously said,
> "I believe the underlying source from where CA keys are loaded might vary
> based on the architecture (".mok" is UEFI specific.). The key part is
> that this new keyring should contain only CA keys which can be later
> used to vouch for user keys loaded onto IMA or secondary keyring at
> runtime. It would be good to have a "ca" in the name, like .xxxx-ca,
> where xxxx can be machine, owner, or system. I prefer .system-ca."
>
> The CA keys on the MOK db is simply the first root of trust being
> defined, but other roots of trust are sure to follow. For this reason,
> I agree naming the new keyring "mok" should be avoided.
As I said previously, I’m open to renaming, I just would like to have an
agreement on the new name before changing everything. The current proposed
names I have heard are “.machine" and ".system-ca". Is there a preference
the maintainers feel is appropriate? If so, please let me know and I’ll
rename it. Thanks.
^ permalink raw reply
* Re: [PATCH] ima: fix infinite loop within "ima_match_policy" function.
From: Mimi Zohar @ 2021-08-19 13:47 UTC (permalink / raw)
To: THOBY Simon, liqiong
Cc: dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <8d17f252-4a93-f430-3f25-e75556ab01e8@viveris.fr>
On Thu, 2021-08-19 at 12:58 +0000, THOBY Simon wrote:
> Hi Liqiong,
>
> On 8/19/21 12:15 PM, liqiong wrote:
> > When "ima_match_policy" is looping while "ima_update_policy" changs
> > the variable "ima_rules", then "ima_match_policy" may can't exit loop,
> > and kernel keeps printf "rcu_sched detected stall on CPU ...".
> >
> > It occurs at boot phase, systemd-services are being checked within
> > "ima_match_policy,at the same time, the variable "ima_rules"
> > is changed by a service.
>
> First off, thanks for finding and identifying this nasty bug.
Once the initial builtin policy rules have been replaced by a custom
policy, rules may only be appended by splicing the new rules with the
existing rules. There should never be a problem reading the rules at
that point. Does this problem occur before the builtin policy rules
have been replaced with a custom policy?
Mimi
^ permalink raw reply
* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
From: Mimi Zohar @ 2021-08-19 13:10 UTC (permalink / raw)
To: Jarkko Sakkinen, Eric Snowberg, keyrings, linux-integrity,
dhowells, dwmw2, herbert, davem, jmorris, serge
Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
linux-security-module, James.Bottomley, pjones, konrad.wilk,
Patrick Uiterwijk
In-Reply-To: <fcb30226f378ef12cd8bd15938f0af0e1a3977a2.camel@kernel.org>
On Thu, 2021-08-19 at 14:38 +0300, Jarkko Sakkinen wrote:
> On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> > Many UEFI Linux distributions boot using shim. The UEFI shim provides
> > what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
> > Boot DB and MOK keys to validate the next step in the boot chain. The
> > MOK facility can be used to import user generated keys. These keys can
> > be used to sign an end-user development kernel build. When Linux boots,
> > pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
> > Linux .platform keyring.
> >
> > Currently, pre-boot keys are not trusted within the Linux trust boundary
> > [1]. These platform keys can only be used for kexec. If an end-user
> > wants to use their own key within the Linux trust boundary, they must
> > either compile it into the kernel themselves or use the insert-sys-cert
> > script. Both options present a problem. Many end-users do not want to
> > compile their own kernels. With the insert-sys-cert option, there are
> > missing upstream changes [2]. Also, with the insert-sys-cert option,
> > the end-user must re-sign their kernel again with their own key, and
> > then insert that key into the MOK db. Another problem with
> > insert-sys-cert is that only a single key can be inserted into a
> > compressed kernel.
> >
> > Having the ability to insert a key into the Linux trust boundary opens
> > up various possibilities. The end-user can use a pre-built kernel and
> > sign their own kernel modules. It also opens up the ability for an
> > end-user to more easily use digital signature based IMA-appraisal. To
> > get a key into the ima keyring, it must be signed by a key within the
> > Linux trust boundary.
>
> As of today, I can use a prebuilt kernel, crate my own MOK key and sign
> modules. What will be different?
The UEFI db and MOK keys are being loaded onto the .platform keyring,
which is suppose to be limited to verifying the kexec kernel image
signature. With a downstream patch, kernel modules are being verified
as well.
Initially Patrick Uiterwijk's "[PATCH 0/3] Load keys from TPM2 NV Index
on IMA keyring" patch set attempted to define a new root of trust based
on a key stored in the TPM. This patch set is similarly attempting to
define a new root of trust based on CA keys stored in the MOK db.
The purpose of this patch set is to define a new, safe trust source
parallel to the builtin keyring, without relying on a downstream patch.
With the new root of trust, the end user could sign his own kernel
modules, sign third party keys, and load keys onto the IMA keyring,
which can be used for signing the IMA policy and other files.
>
> > Downstream Linux distros try to have a single signed kernel for each
> > architecture. Each end-user may use this kernel in entirely different
> > ways. Some downstream kernels have chosen to always trust platform keys
> > within the Linux trust boundary for kernel module signing. These
> > kernels have no way of using digital signature base IMA appraisal.
> >
> > This series introduces a new Linux kernel keyring containing the Machine
> > Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
>
> I would name it as ".machine" because it is more "re-usable" name, e.g.
> could be used for similar things as MOK. ".mok" is a bad name because
> it binds directly to a single piece of user space software.
Nayna previously said,
"I believe the underlying source from where CA keys are loaded might vary
based on the architecture (".mok" is UEFI specific.). The key part is
that this new keyring should contain only CA keys which can be later
used to vouch for user keys loaded onto IMA or secondary keyring at
runtime. It would be good to have a "ca" in the name, like .xxxx-ca,
where xxxx can be machine, owner, or system. I prefer .system-ca."
The CA keys on the MOK db is simply the first root of trust being
defined, but other roots of trust are sure to follow. For this reason,
I agree naming the new keyring "mok" should be avoided.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH 3/3] virt: Add sev_secret module to expose confidential computing secrets
From: Andrew Scull @ 2021-08-19 13:02 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Dov Murik, linux-efi, Borislav Petkov, Ashish Kalra,
Brijesh Singh, Tom Lendacky, James Morris, Serge E. Hallyn,
Andi Kleen, Dr. David Alan Gilbert, James Bottomley,
Tobin Feldman-Fitzthum, Jim Cadden, linux-coco,
linux-security-module, Linux Kernel Mailing List
In-Reply-To: <CAMj1kXFC-cizTw2Tv40uZHdLArKtdMNxdQXWoPWSL-8qexdkLQ@mail.gmail.com>
On Mon, 16 Aug 2021 at 10:57, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Fri, 13 Aug 2021 at 15:05, Andrew Scull <ascull@google.com> wrote:
> >
> > On Mon, Aug 09, 2021 at 07:01:57PM +0000, Dov Murik wrote:
> > > The new sev_secret module exposes the confidential computing (coco)
> > > secret area via securityfs interface.
> > >
> > > When the module is loaded (and securityfs is mounted, typically under
> > > /sys/kernel/security), a "coco/sev_secret" directory is created in
> > > securityfs. In it, a file is created for each secret entry. The name
> > > of each such file is the GUID of the secret entry, and its content is
> > > the secret data.
> > >
> > > This allows applications running in a confidential computing setting to
> > > read secrets provided by the guest owner via a secure secret injection
> > > mechanism (such as AMD SEV's LAUNCH_SECRET command).
> > >
> > > Removing (unlinking) files in the "coco/sev_secret" directory will zero
> > > out the secret in memory, and remove the filesystem entry. If the
> > > module is removed and loaded again, that secret will not appear in the
> > > filesystem.
> >
> > We've also been looking into a similar secret mechanism recently in the
> > context of Android and protected KVM [1]. Our secrets would come from a
> > different source, likely described as a reserved-memory node in the DT,
> > but would need to be exposed to userspace in the same way as the SEV
> > secrets. Originally I tried using a character device, but this approach
> > with securityfs feels neater to me.
> >
>
> Agreed. I particularly like how deleting the file wipes the secret from memory.
>
> > We're also looking to pass secrets from the bootloader to Linux, outside
> > of any virtualization or confidential compute context (at least a far as
> > I have understood the meaning of the term). Again, this feels like it
> > would be exposed to userspace in the same way.
> >
>
> Indeed.
>
> > It would be good to be able to share the parts that would be common. I
> > expect that would mean the operations for a secret file and for a
> > directory of secrets at a minimum. But it might also influence the paths
> > in securityfs; I see, looking back, that the "coco" directory was added
> > since the RFC but would a generalized "secret" subsystem make sense? Or
> > would it be preferable for each case to define their own path?
> >
>
> I think we should avoid 'secret', to be honest. Even if protected KVM
> is not riding the SEV/TDX wave, I think confidential computing is
> still an accurate description of its semantics.
I agree that protected KVM fits with the ideas of confidential
computing. It was the non-virtualization context that I was less
certain about. For example, the Open Profile for DICE [2] starts with
a hardware secret and derives, at each boot stage, a secret that is
passed to the next stage. It's a process that applies both to a VM,
matching confidential compute as I understand it, but also the host
Linux, which is the part that I wasn't so clear on.
[2] -- https://pigweed.googlesource.com/open-dice/+/refs/heads/main/docs/specification.md
> > [1] -- https://lwn.net/Articles/836693/
> >
> > > +static int sev_secret_unlink(struct inode *dir, struct dentry *dentry)
> > > +{
> > > + struct sev_secret *s = sev_secret_get();
> > > + struct inode *inode = d_inode(dentry);
> > > + struct secret_entry *e = (struct secret_entry *)inode->i_private;
> > > + int i;
> > > +
> > > + if (e) {
> > > + /* Zero out the secret data */
> > > + memzero_explicit(e->data, secret_entry_data_len(e));
> >
> > Would there be a benefit in flushing these zeros?
> >
>
> Do you mean cache clean+invalidate? Better to be precise here.
At least a clean, to have the zeros written back to memory from the
cache, in order to overwrite the secret.
>
> > > + e->guid = NULL_GUID;
> > > + }
> > > +
> > > + inode->i_private = NULL;
> > > +
> > > + for (i = 0; i < SEV_SECRET_NUM_FILES; i++)
> > > + if (s->fs_files[i] == dentry)
> > > + s->fs_files[i] = NULL;
> > > +
> > > + /*
> > > + * securityfs_remove tries to lock the directory's inode, but we reach
> > > + * the unlink callback when it's already locked
> > > + */
> > > + inode_unlock(dir);
> > > + securityfs_remove(dentry);
> > > + inode_lock(dir);
> > > +
> > > + return 0;
> > > +}
^ permalink raw reply
* Re: [PATCH] ima: fix infinite loop within "ima_match_policy" function.
From: THOBY Simon @ 2021-08-19 12:58 UTC (permalink / raw)
To: liqiong, zohar@linux.ibm.com
Cc: dmitry.kasatkin@gmail.com, jmorris@namei.org, serge@hallyn.com,
linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20210819101529.28001-1-liqiong@nfschina.com>
Hi Liqiong,
On 8/19/21 12:15 PM, liqiong wrote:
> When "ima_match_policy" is looping while "ima_update_policy" changs
> the variable "ima_rules", then "ima_match_policy" may can't exit loop,
> and kernel keeps printf "rcu_sched detected stall on CPU ...".
>
> It occurs at boot phase, systemd-services are being checked within
> "ima_match_policy,at the same time, the variable "ima_rules"
> is changed by a service.
First off, thanks for finding and identifying this nasty bug.
>
> Signed-off-by: liqiong <liqiong@nfschina.com>
> ---
> security/integrity/ima/ima_policy.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index fd5d46e511f1..7e71e643457c 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -217,6 +217,7 @@ static LIST_HEAD(ima_default_rules);
> static LIST_HEAD(ima_policy_rules);
> static LIST_HEAD(ima_temp_rules);
> static struct list_head *ima_rules = &ima_default_rules;
> +static DECLARE_RWSEM(ima_rules_sem);
>
> static int ima_policy __initdata;
>
> @@ -666,6 +667,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
> if (template_desc && !*template_desc)
> *template_desc = ima_template_desc_current();
>
> + down_read(&ima_rules_sem);
> rcu_read_lock();
> list_for_each_entry_rcu(entry, ima_rules, list) {
>
> @@ -702,6 +704,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
> break;
> }
> rcu_read_unlock();
> + up_read(&ima_rules_sem);
>
> return action;
> }
> @@ -919,7 +922,9 @@ void ima_update_policy(void)
>
> if (ima_rules != policy) {
> ima_policy_flag = 0;
> + down_write(&ima_rules_sem);
> ima_rules = policy;
> + up_write(&ima_rules_sem);
>
> /*
> * IMA architecture specific policy rules are specified
>
Rather than introducing a new semaphore, I wonder if you couldn't have done something
like the following?
@@ -674,13 +674,15 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
const char *func_data, unsigned int *allowed_algos)
{
struct ima_rule_entry *entry;
+ struct list_head *ima_rules_tmp;
int action = 0, actmask = flags | (flags << 1);
if (template_desc && !*template_desc)
*template_desc = ima_template_desc_current();
rcu_read_lock();
- list_for_each_entry_rcu(entry, ima_rules, list) {
+ ima_rules_tmp = rcu_dereference(ima_rules);
+ list_for_each_entry_rcu(entry, ima_rules_tmp, list) {
if (!(entry->action & actmask))
continue;
@@ -970,7 +972,7 @@ void ima_update_policy(void)
if (ima_rules != policy) {
ima_policy_flag = 0;
- ima_rules = policy;
+ rcu_assign_pointer(ima_rules, policy);
/*
* IMA architecture specific policy rules are specified
Also, ima_match_policy is not the only place where we iterate over ima_rules, maybe
this change should be applied to every function that perform a call the like of
"list_for_each_entry_rcu(entry, ima_rules_tmp, list)" ?
All that being said, your change is quite small and I have no objection to it,
I was just wondering whether we could achieve the same effect without locks
with RCU.
What do you think?
Thanks,
Simon
^ permalink raw reply
* Re: [PATCH v4 00/12] Enroll kernel keys thru MOK
From: Jarkko Sakkinen @ 2021-08-19 11:38 UTC (permalink / raw)
To: Eric Snowberg, keyrings, linux-integrity, zohar, dhowells, dwmw2,
herbert, davem, jmorris, serge
Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
linux-security-module, James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
On Wed, 2021-08-18 at 20:20 -0400, Eric Snowberg wrote:
> Many UEFI Linux distributions boot using shim. The UEFI shim provides
> what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
> Boot DB and MOK keys to validate the next step in the boot chain. The
> MOK facility can be used to import user generated keys. These keys can
> be used to sign an end-user development kernel build. When Linux boots,
> pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
> Linux .platform keyring.
>
> Currently, pre-boot keys are not trusted within the Linux trust boundary
> [1]. These platform keys can only be used for kexec. If an end-user
> wants to use their own key within the Linux trust boundary, they must
> either compile it into the kernel themselves or use the insert-sys-cert
> script. Both options present a problem. Many end-users do not want to
> compile their own kernels. With the insert-sys-cert option, there are
> missing upstream changes [2]. Also, with the insert-sys-cert option,
> the end-user must re-sign their kernel again with their own key, and
> then insert that key into the MOK db. Another problem with
> insert-sys-cert is that only a single key can be inserted into a
> compressed kernel.
>
> Having the ability to insert a key into the Linux trust boundary opens
> up various possibilities. The end-user can use a pre-built kernel and
> sign their own kernel modules. It also opens up the ability for an
> end-user to more easily use digital signature based IMA-appraisal. To
> get a key into the ima keyring, it must be signed by a key within the
> Linux trust boundary.
As of today, I can use a prebuilt kernel, crate my own MOK key and sign
modules. What will be different?
> Downstream Linux distros try to have a single signed kernel for each
> architecture. Each end-user may use this kernel in entirely different
> ways. Some downstream kernels have chosen to always trust platform keys
> within the Linux trust boundary for kernel module signing. These
> kernels have no way of using digital signature base IMA appraisal.
>
> This series introduces a new Linux kernel keyring containing the Machine
> Owner Keys (MOK) called .mok. It also adds a new MOK variable to shim.
I would name it as ".machine" because it is more "re-usable" name, e.g.
could be used for similar things as MOK. ".mok" is a bad name because
it binds directly to a single piece of user space software.
/Jarkko
^ permalink raw reply
* [PATCH] ima: fix infinite loop within "ima_match_policy" function.
From: liqiong @ 2021-08-19 10:15 UTC (permalink / raw)
To: zohar
Cc: dmitry.kasatkin, jmorris, serge, linux-integrity,
linux-security-module, linux-kernel, liqiong
When "ima_match_policy" is looping while "ima_update_policy" changs
the variable "ima_rules", then "ima_match_policy" may can't exit loop,
and kernel keeps printf "rcu_sched detected stall on CPU ...".
It occurs at boot phase, systemd-services are being checked within
"ima_match_policy,at the same time, the variable "ima_rules"
is changed by a service.
Signed-off-by: liqiong <liqiong@nfschina.com>
---
security/integrity/ima/ima_policy.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index fd5d46e511f1..7e71e643457c 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -217,6 +217,7 @@ static LIST_HEAD(ima_default_rules);
static LIST_HEAD(ima_policy_rules);
static LIST_HEAD(ima_temp_rules);
static struct list_head *ima_rules = &ima_default_rules;
+static DECLARE_RWSEM(ima_rules_sem);
static int ima_policy __initdata;
@@ -666,6 +667,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
if (template_desc && !*template_desc)
*template_desc = ima_template_desc_current();
+ down_read(&ima_rules_sem);
rcu_read_lock();
list_for_each_entry_rcu(entry, ima_rules, list) {
@@ -702,6 +704,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
break;
}
rcu_read_unlock();
+ up_read(&ima_rules_sem);
return action;
}
@@ -919,7 +922,9 @@ void ima_update_policy(void)
if (ima_rules != policy) {
ima_policy_flag = 0;
+ down_write(&ima_rules_sem);
ima_rules = policy;
+ up_write(&ima_rules_sem);
/*
* IMA architecture specific policy rules are specified
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes
From: Casey Schaufler @ 2021-08-19 0:56 UTC (permalink / raw)
To: Paul Moore
Cc: casey.schaufler, James Morris, linux-security-module, selinux,
linux-audit, keescook, john.johansen, penguin-kernel,
Stephen Smalley, Casey Schaufler
In-Reply-To: <CAHC9VhT=QL5pKekaPB-=LDzU3hck9nXDiL5n1-upSqPg3gq=7w@mail.gmail.com>
On 8/18/2021 5:47 PM, Paul Moore wrote:
> ...
> I just spent a few minutes tracing the code paths up from audit
> through netlink and then through the socket layer and I'm not seeing
> anything obvious where the path differs from any other syscall;
> current->audit_context *should* be valid just like any other syscall.
> However, I do have to ask, are you only seeing these audit records
> with a current->audit_context equal to NULL during early boot?
Nope. Sorry.
^ permalink raw reply
* Re: [PATCH v28 22/25] Audit: Add record for multiple process LSM attributes
From: Paul Moore @ 2021-08-19 0:47 UTC (permalink / raw)
To: Casey Schaufler
Cc: casey.schaufler, James Morris, linux-security-module, selinux,
linux-audit, keescook, john.johansen, penguin-kernel,
Stephen Smalley
In-Reply-To: <062ba5f9-e4e8-31f4-7815-826f44b35654@schaufler-ca.com>
On Wed, Aug 18, 2021 at 5:59 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 8/16/2021 11:57 AM, Paul Moore wrote:
> > On Fri, Aug 13, 2021 at 5:47 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >> On 8/13/2021 1:43 PM, Paul Moore wrote:
> ...
> > Yeah, the thought occurred to me, but we are clearly already in the
> > maybe-the-assumptions-are-wrong stage so I'm not going to rely on that
> > being 100%. We definitely need to track this down before we start
> > making to many more guesses about what is working and what is not.
>
> I've been tracking down where the audit context isn't set where
> we'd expect it to be, I've identified 5 cases:
>
> 1000 AUDIT_GET - Get Status
> 1001 AUDIT_SET - Set status enable/disable/auditd
> 1010 AUDIT_SIGNAL_INFO
> 1130 AUDIT_SERVICE_START
> 1131 AUDIT_SEVICE_STOP
>
> These are all events that relate to the audit system itself.
> It seems plausible that these really aren't syscalls and hence
> shouldn't be expected to have an audit_context. I will create a
> patch that treats these as the special cases I believe them to be.
Yes, all but two of these could be considered to be audit subsystem
control messages, but AUDIT_SERVICE_{START,STOP} I think definitely
fall outside the audit subsystem control message category. The
AUDIT_SERVICE_{START,STOP} records are used to indicate when a
service, e.g. NetworkManager, starts and stops; on my fedora test
system they are generated by systemd since it manages service state on
that system; a quick example is below, but I'm sure you've seen plenty
of these already.
% ausearch -m SERVICE_START
time->Wed Aug 18 20:13:00 2021
type=SERVICE_START msg=audit(1629331980.779:1186): pid=1 uid=0 auid=4294967295 s
es=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=NetworkManager-dispatch
er comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? re
s=success'
However, regardless of if the message is related to controlling the
audit subsystem or not, we do want to be able to associate those
records with other related records, e.g. SYSCALL records. Looking at
the message types you listed above, they are all records that are
triggered by userspace via netlink messages; if you haven't already I
would start poking along that code path to see if something looks
awry.
I just spent a few minutes tracing the code paths up from audit
through netlink and then through the socket layer and I'm not seeing
anything obvious where the path differs from any other syscall;
current->audit_context *should* be valid just like any other syscall.
However, I do have to ask, are you only seeing these audit records
with a current->audit_context equal to NULL during early boot?
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH v4 12/12] integrity: Only use mok keyring when uefi_check_trust_mok_keys is true
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
With the introduction of uefi_check_trust_mok_keys, it signifies the end-
user wants to trust the mok keyring as trusted keys. If they have chosen
to trust the mok keyring, load the qualifying keys into it during boot,
then link it to the secondary keyring . If the user has not chosen to
trust the mok keyring, it will be empty and not linked to the secondary
keyring.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v4: Initial version
---
security/integrity/digsig.c | 2 +-
security/integrity/integrity.h | 5 +++++
.../integrity/platform_certs/keyring_handler.c | 2 +-
security/integrity/platform_certs/mok_keyring.c | 16 ++++++++++++++++
4 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 0f14ffef9c43..fd255e5b6293 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -116,7 +116,7 @@ static int __init __integrity_init_keyring(const unsigned int id,
} else {
if (id == INTEGRITY_KEYRING_PLATFORM)
set_platform_trusted_keys(keyring[id]);
- if (id == INTEGRITY_KEYRING_MOK)
+ if (id == INTEGRITY_KEYRING_MOK && trust_moklist())
set_mok_trusted_keys(keyring[id]);
if (id == INTEGRITY_KEYRING_IMA)
load_module_cert(keyring[id]);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index be56ba49dc19..57683fdea2af 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -287,9 +287,14 @@ static inline void __init add_to_platform_keyring(const char *source,
#ifdef CONFIG_INTEGRITY_MOK_KEYRING
void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+bool __init trust_moklist(void);
#else
static inline void __init add_to_mok_keyring(const char *source,
const void *data, size_t len)
{
}
+static inline bool __init trust_moklist(void)
+{
+ return false;
+}
#endif
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index fc4ad85d9223..471bf103b444 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -82,7 +82,7 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
{
if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
- if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING))
+ if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING) && trust_moklist())
return add_to_mok_keyring;
else
return add_to_platform_keyring;
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index bcfab894a9dc..3dbb6d17e17d 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -8,6 +8,8 @@
#include <linux/efi.h>
#include "../integrity.h"
+bool trust_mok;
+
static __init int mok_keyring_init(void)
{
int rc;
@@ -67,3 +69,17 @@ static __init bool uefi_check_trust_mok_keys(void)
*/
return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
}
+
+bool __init trust_moklist(void)
+{
+ static bool initialized;
+
+ if (!initialized) {
+ initialized = true;
+
+ if (uefi_check_trust_mok_keys())
+ trust_mok = true;
+ }
+
+ return trust_mok;
+}
--
2.18.4
^ permalink raw reply related
* [PATCH v4 11/12] integrity: Trust MOK keys if MokListTrustedRT found
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
A new Machine Owner Key (MOK) variable called MokListTrustedRT has been
introduced in shim. When this UEFI variable is set, it indicates the
end-user has made the decision themself that they wish to trust MOK keys
within the Linux trust boundary. It is not an error if this variable
does not exist. If it does not exist, the MOK keys should not be trusted
within the kernel.
MOK variables are mirrored from Boot Services to Runtime Services. When
shim sees the new MokTML BS variable, it will create a new variable
(before Exit Boot Services is called) called MokListTrustedRT without
EFI_VARIABLE_NON_VOLATILE set. Following Exit Boot Services, UEFI
variables can only be set and created with SetVariable if both
EFI_VARIABLE_RUNTIME_ACCESS & EFI_VARIABLE_NON_VOLATILE are set.
Therefore, this can not be defeated by simply creating a
MokListTrustedRT variable from Linux, the existence of
EFI_VARIABLE_NON_VOLATILE will cause uefi_check_trust_mok_keys to return
false.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed mok_keyring_trust_setup function
v4: Unmodified from v2
---
.../integrity/platform_certs/mok_keyring.c | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index bcd9ac78ce3b..bcfab894a9dc 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -5,6 +5,7 @@
* Copyright (c) 2021, Oracle and/or its affiliates.
*/
+#include <linux/efi.h>
#include "../integrity.h"
static __init int mok_keyring_init(void)
@@ -40,3 +41,29 @@ void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
if (rc)
pr_info("Error adding keys to mok keyring %s\n", source);
}
+
+/*
+ * Try to load the MokListTrustedRT UEFI variable to see if we should trust
+ * the mok keys within the kernel. It is not an error if this variable
+ * does not exist. If it does not exist, mok keys should not be trusted
+ * within the kernel.
+ */
+static __init bool uefi_check_trust_mok_keys(void)
+{
+ efi_status_t status;
+ unsigned int mtrust = 0;
+ unsigned long size = sizeof(mtrust);
+ efi_guid_t guid = EFI_SHIM_LOCK_GUID;
+ u32 attr;
+
+ status = efi.get_variable(L"MokListTrustedRT", &guid, &attr, &size, &mtrust);
+
+ /*
+ * The EFI_VARIABLE_NON_VOLATILE check is to verify MokListTrustedRT
+ * was set thru shim mirrioring and not by a user from the host os.
+ * According to the UEFI spec, once EBS is performed, SetVariable()
+ * will succeed only when both EFI_VARIABLE_RUNTIME_ACCESS &
+ * EFI_VARIABLE_NON_VOLATILE are set.
+ */
+ return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
+}
--
2.18.4
^ permalink raw reply related
* [PATCH v4 10/12] integrity: store reference to mok keyring
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Store a reference to the mok keyring in system keyring code. The
system keyring code needs this to complete the keyring link to
to mok keyring.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: Unmodified from v2
v4: Removed trust_moklist check
---
security/integrity/digsig.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index a93d558b795b..0f14ffef9c43 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -116,6 +116,8 @@ static int __init __integrity_init_keyring(const unsigned int id,
} else {
if (id == INTEGRITY_KEYRING_PLATFORM)
set_platform_trusted_keys(keyring[id]);
+ if (id == INTEGRITY_KEYRING_MOK)
+ set_mok_trusted_keys(keyring[id]);
if (id == INTEGRITY_KEYRING_IMA)
load_module_cert(keyring[id]);
}
--
2.18.4
^ permalink raw reply related
* [PATCH v4 09/12] KEYS: link secondary_trusted_keys to mok trusted keys
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Allow the .mok keyring to be linked to the secondary_trusted_keys. After
the link is created, keys contained in the .mok keyring will automatically
be searched when searching secondary_trusted_keys.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
v4: Unmodified from v3
---
certs/system_keyring.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 1c39af137cf1..4ce39b4ccc04 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -101,6 +101,9 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
void __init set_mok_trusted_keys(struct key *keyring)
{
mok_trusted_keys = keyring;
+
+ if (key_link(secondary_trusted_keys, mok_trusted_keys) < 0)
+ panic("Can't link (mok) trusted keyrings\n");
}
/**
--
2.18.4
^ permalink raw reply related
* [PATCH v4 02/12] integrity: Do not allow mok keyring updates following init
From: Eric Snowberg @ 2021-08-19 0:20 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
The mok keyring is setup during init. No additional keys should be allowed
to be added afterwards. Leave the permission as read only.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v4: Unmodified from v2
---
security/integrity/digsig.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e07334504ef1..5cad38e6f56a 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -140,7 +140,8 @@ int __init integrity_init_keyring(const unsigned int id)
return -ENOMEM;
restriction->check = restrict_link_to_ima;
- perm |= KEY_USR_WRITE;
+ if (id != INTEGRITY_KEYRING_MOK)
+ perm |= KEY_USR_WRITE;
out:
return __integrity_init_keyring(id, perm, restriction);
--
2.18.4
^ permalink raw reply related
* [PATCH v4 04/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Set the restriction check for INTEGRITY_KEYRING_MOK keys to
restrict_link_by_ca. This will only allow CA keys into the mok
keyring.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Added !IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING check so mok
keyring gets created even when it isn't enabled
v3: Rename restrict_link_by_system_trusted_or_ca to restrict_link_by_ca
v4: removed unnecessary restriction->check set
---
security/integrity/digsig.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 5cad38e6f56a..1f410242752c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -132,14 +132,18 @@ int __init integrity_init_keyring(const unsigned int id)
goto out;
}
- if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING))
+ if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING) && id != INTEGRITY_KEYRING_MOK)
return 0;
restriction = kzalloc(sizeof(struct key_restriction), GFP_KERNEL);
if (!restriction)
return -ENOMEM;
- restriction->check = restrict_link_to_ima;
+ if (id == INTEGRITY_KEYRING_MOK)
+ restriction->check = restrict_link_by_ca;
+ else
+ restriction->check = restrict_link_to_ima;
+
if (id != INTEGRITY_KEYRING_MOK)
perm |= KEY_USR_WRITE;
--
2.18.4
^ permalink raw reply related
* [PATCH v4 08/12] KEYS: integrity: change link restriction to trust the mok keyring
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
With the introduction of the mok keyring, the end-user may choose to
trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
trust them, the .mok keyring will contain these keys. If not, the mok
keyring will always be empty. Update the restriction check to allow the
secondary trusted keyring and ima keyring to also trust mok keys.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v4: Initial version (consolidated two previous patches)
---
certs/system_keyring.c | 5 ++++-
security/integrity/digsig.c | 4 ++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index a75c815a42c8..1c39af137cf1 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -89,7 +89,10 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
if (!restriction)
panic("Can't allocate secondary trusted keyring restriction\n");
- restriction->check = restrict_link_by_builtin_and_secondary_trusted;
+ if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING))
+ restriction->check = restrict_link_by_builtin_secondary_and_ca_trusted;
+ else
+ restriction->check = restrict_link_by_builtin_and_secondary_trusted;
return restriction;
}
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 1f410242752c..a93d558b795b 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -34,7 +34,11 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
};
#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+#define restrict_link_to_ima restrict_link_by_builtin_secondary_and_ca_trusted
+#else
#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
+#endif
#else
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
--
2.18.4
^ permalink raw reply related
* [PATCH v4 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
From: Eric Snowberg @ 2021-08-19 0:20 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Many UEFI Linux distributions boot using shim. The UEFI shim provides
what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
Boot DB and MOK keys to validate the next step in the boot chain. The
MOK facility can be used to import user generated keys. These keys can
be used to sign an end-users development kernel build. When Linux
boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux
.platform keyring.
Add a new Linux keyring called .mok. This keyring shall contain just
MOK CA keys and not the remaining keys in the platform keyring. This new
.mok keyring will be used in follow on patches. Unlike keys in the
platform keyring, keys contained in the .mok keyring will be trusted
within the kernel if the end-user has chosen to do so.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed destory keyring code
v3: Unmodified from v2
v4: Add Kconfig, merged in "integrity: add add_to_mok_keyring"
---
security/integrity/Kconfig | 11 +++++
security/integrity/Makefile | 1 +
security/integrity/digsig.c | 1 +
security/integrity/integrity.h | 12 +++++-
.../integrity/platform_certs/mok_keyring.c | 42 +++++++++++++++++++
5 files changed, 66 insertions(+), 1 deletion(-)
create mode 100644 security/integrity/platform_certs/mok_keyring.c
diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 71f0177e8716..7a69021e2d42 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -62,6 +62,17 @@ config INTEGRITY_PLATFORM_KEYRING
provided by the platform for verifying the kexec'ed kerned image
and, possibly, the initramfs signature.
+config INTEGRITY_MOK_KEYRING
+ bool "Provide a keyring to which CA Machine Owner Keys may be added"
+ depends on SECONDARY_TRUSTED_KEYRING
+ depends on INTEGRITY_ASYMMETRIC_KEYS
+ depends on SYSTEM_BLACKLIST_KEYRING
+ help
+ If set, provide a keyring to which CA Machine Owner Keys (MOK) may
+ be added. This keyring shall contain just CA MOK keys. Unlike keys
+ in the platform keyring, keys contained in the .mok keyring will be
+ trusted within the kernel.
+
config LOAD_UEFI_KEYS
depends on INTEGRITY_PLATFORM_KEYRING
depends on EFI
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 7ee39d66cf16..e3f4588a069c 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -10,6 +10,7 @@ integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
+integrity-$(CONFIG_INTEGRITY_MOK_KEYRING) += platform_certs/mok_keyring.o
integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
platform_certs/load_uefi.o \
platform_certs/keyring_handler.o
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 3b06a01bd0fd..e07334504ef1 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -30,6 +30,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
".ima",
#endif
".platform",
+ ".mok",
};
#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 547425c20e11..be56ba49dc19 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -151,7 +151,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
#define INTEGRITY_KEYRING_EVM 0
#define INTEGRITY_KEYRING_IMA 1
#define INTEGRITY_KEYRING_PLATFORM 2
-#define INTEGRITY_KEYRING_MAX 3
+#define INTEGRITY_KEYRING_MOK 3
+#define INTEGRITY_KEYRING_MAX 4
extern struct dentry *integrity_dir;
@@ -283,3 +284,12 @@ static inline void __init add_to_platform_keyring(const char *source,
{
}
#endif
+
+#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+#else
+static inline void __init add_to_mok_keyring(const char *source,
+ const void *data, size_t len)
+{
+}
+#endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
new file mode 100644
index 000000000000..bcd9ac78ce3b
--- /dev/null
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -0,0 +1,42 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MOK keyring routines.
+ *
+ * Copyright (c) 2021, Oracle and/or its affiliates.
+ */
+
+#include "../integrity.h"
+
+static __init int mok_keyring_init(void)
+{
+ int rc;
+
+ rc = integrity_init_keyring(INTEGRITY_KEYRING_MOK);
+ if (rc)
+ return rc;
+
+ pr_notice("MOK Keyring initialized\n");
+ return 0;
+}
+device_initcall(mok_keyring_init);
+
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
+{
+ key_perm_t perm;
+ int rc;
+
+ perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
+ rc = integrity_load_cert(INTEGRITY_KEYRING_MOK, source, data, len, perm);
+
+ /*
+ * Some MOKList keys may not pass the mok keyring restrictions.
+ * If the restriction check does not pass and the platform keyring
+ * is configured, try to add it into that keyring instead.
+ */
+ if (rc)
+ rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source,
+ data, len, perm);
+
+ if (rc)
+ pr_info("Error adding keys to mok keyring %s\n", source);
+}
--
2.18.4
^ permalink raw reply related
* [PATCH v4 05/12] integrity: add new keyring handler for mok keys
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Currently both Secure Boot DB and Machine Owner Keys (MOK) go through
the same keyring handler (get_handler_for_db). With the addition of the
new mok keyring, the end-user may choose to trust MOK keys.
Introduce a new keyring handler specific for mok keys. If mok keys are
trusted by the end-user, use the new keyring handler instead.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v3: Only change the keyring handler if the secondary is enabled
v4: Removed trust_moklist check
---
.../integrity/platform_certs/keyring_handler.c | 17 ++++++++++++++++-
.../integrity/platform_certs/keyring_handler.h | 5 +++++
security/integrity/platform_certs/load_uefi.c | 4 ++--
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index 5604bd57c990..fc4ad85d9223 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -66,7 +66,7 @@ static __init void uefi_revocation_list_x509(const char *source,
/*
* Return the appropriate handler for particular signature list types found in
- * the UEFI db and MokListRT tables.
+ * the UEFI db tables.
*/
__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
{
@@ -75,6 +75,21 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
return 0;
}
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
+ if (IS_ENABLED(CONFIG_INTEGRITY_MOK_KEYRING))
+ return add_to_mok_keyring;
+ else
+ return add_to_platform_keyring;
+ }
+ return 0;
+}
+
/*
* Return the appropriate handler for particular signature list types found in
* the UEFI dbx and MokListXRT tables.
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 2462bfa08fe3..284558f30411 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -24,6 +24,11 @@ void blacklist_binary(const char *source, const void *data, size_t len);
*/
efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
+/*
+ * Return the handler for particular signature list types found in the mok.
+ */
+efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
+
/*
* Return the handler for particular signature list types found in the dbx.
*/
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index f290f78c3f30..c1bfd1cd7cc3 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -94,7 +94,7 @@ static int __init load_moklist_certs(void)
rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
mokvar_entry->data,
mokvar_entry->data_size,
- get_handler_for_db);
+ get_handler_for_mok);
/* All done if that worked. */
if (!rc)
return rc;
@@ -109,7 +109,7 @@ static int __init load_moklist_certs(void)
mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
if (mok) {
rc = parse_efi_signature_list("UEFI:MokListRT",
- mok, moksize, get_handler_for_db);
+ mok, moksize, get_handler_for_mok);
kfree(mok);
if (rc)
pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
--
2.18.4
^ permalink raw reply related
* [PATCH v4 07/12] KEYS: Introduce link restriction to include builtin, secondary and mok keys
From: Eric Snowberg @ 2021-08-19 0:21 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, konrad.wilk
In-Reply-To: <20210819002109.534600-1-eric.snowberg@oracle.com>
Introduce a new link restriction that includes the trusted builtin,
secondary and mok keys. The restriction is based on the key to be added
being vouched for by a key in any of these three keyrings.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
v4: moved code under CONFIG_INTEGRITY_MOK_KEYRING
---
certs/system_keyring.c | 23 +++++++++++++++++++++++
include/keys/system_keyring.h | 6 ++++++
2 files changed, 29 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 94af3fe107b4..a75c815a42c8 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -99,6 +99,29 @@ void __init set_mok_trusted_keys(struct key *keyring)
{
mok_trusted_keys = keyring;
}
+
+/**
+ * restrict_link_by_builtin_secondary_and_ca_trusted
+ *
+ * Restrict the addition of keys into a keyring based on the key-to-be-added
+ * being vouched for by a key in either the built-in, the secondary, or
+ * the mok keyrings.
+ */
+int restrict_link_by_builtin_secondary_and_ca_trusted(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key)
+{
+ if (mok_trusted_keys && type == &key_type_keyring &&
+ dest_keyring == secondary_trusted_keys &&
+ payload == &mok_trusted_keys->payload)
+ /* Allow the mok keyring to be added to the secondary */
+ return 0;
+
+ return restrict_link_by_builtin_and_secondary_trusted(dest_keyring, type,
+ payload, restrict_key);
+}
#endif
/*
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 059e32e36b3a..8cc9606a6cab 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -39,8 +39,14 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
#endif
#ifdef CONFIG_INTEGRITY_MOK_KEYRING
+extern int restrict_link_by_builtin_secondary_and_ca_trusted(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key);
extern void __init set_mok_trusted_keys(struct key *keyring);
#else
+#define restrict_link_by_builtin_secondary_and_ca_trusted restrict_link_by_builtin_trusted
static inline void __init set_mok_trusted_keys(struct key *keyring)
{
}
--
2.18.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox