2019-03-06 23:59:58

by Matthew Garrett

[permalink] [raw]
Subject: [PULL REQUEST] Kernel lockdown patches for 5.2

Hi James,

This patchset introduces an optional kernel lockdown feature,
intended to strengthen the boundary between UID 0 and the kernel. When
enabled and active (by enabling the config option and passing the
"lockdown" option on the kernel command line), various pieces of
kernel functionality are restricted. Applications that rely on
low-level access to either hardware or the kernel may cease working as
a result - therefore this should not be enabled without appropriate
evaluation beforehand.

The majority of mainstream distributions have been carrying variants
of this patchset for many years now, so there's value in providing a
unified upstream implementation to reduce the delta. This PR probably
doesn't meet every distribution requirement, but gets us much closer
to not requiring external patches.

This PR is mostly the same as the previous attempt, but with the
following changes:

1) The integration between EFI secure boot and the lockdown state has
been removed
2) A new CONFIG_KERNEL_LOCK_DOWN_FORCE kconfig option has been added,
which will always enable lockdown regardless of the kernel command
line
3) The integration with IMA has been dropped for now. IMA is in the
process of adding support for architecture-specific policies that will
interact correctly with the lockdown feature, and a followup patch will
integrate that so we don't end up with an ordering dependency on the
merge

The following changes since commit 468e91cecb3218afd684b8c422490dfebe0691bb:

keys: fix missing __user in KEYCTL_PKEY_QUERY (2019-03-04 15:48:37 -0800)

are available in the Git repository at:

https://github.com/mjg59/linux lock_down

for you to fetch changes up to 3d53449e0ac1df8cfdcc1ec48dc9cb622f220300:

lockdown: Print current->comm in restriction messages (2019-03-06 13:32:19 -0800)

----------------------------------------------------------------
Dave Young (1):
Copy secure_boot flag in boot params across kexec reboot

David Howells (12):
Add the ability to lock down access to the running kernel image
Enforce module signatures if the kernel is locked down
Prohibit PCMCIA CIS storage when the kernel is locked down
Lock down TIOCSSERIAL
Lock down module params that specify hardware parameters (eg. ioport)
x86/mmiotrace: Lock down the testmmiotrace module
Lock down /proc/kcore
Lock down kprobes
bpf: Restrict kernel image access functions when the kernel is locked down
Lock down perf
debugfs: Restrict debugfs when the kernel is locked down
lockdown: Print current->comm in restriction messages

Jiri Bohac (2):
kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE
kexec_file: Restrict at runtime if the kernel is locked down

Josh Boyer (2):
hibernate: Disable when the kernel is locked down
acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

Kyle McMartin (1):
Add a SysRq option to lift kernel lockdown

Linn Crosetto (2):
acpi: Disable ACPI table override if the kernel is locked down
acpi: Disable APEI error injection if the kernel is locked down

Matthew Garrett (7):
Restrict /dev/{mem,kmem,port} when the kernel is locked down
kexec_load: Disable at runtime if the kernel is locked down
uswsusp: Disable when the kernel is locked down
PCI: Lock down BAR access when the kernel is locked down
x86: Lock down IO port access when the kernel is locked down
x86/msr: Restrict MSR access when the kernel is locked down
ACPI: Limit access to custom_method when the kernel is locked down

arch/x86/Kconfig | 20 +++++--
arch/x86/include/asm/setup.h | 2 +
arch/x86/kernel/ioport.c | 6 +-
arch/x86/kernel/kexec-bzimage64.c | 1 +
arch/x86/kernel/msr.c | 10 ++++
arch/x86/mm/testmmiotrace.c | 3 +
crypto/asymmetric_keys/verify_pefile.c | 4 +-
drivers/acpi/apei/einj.c | 3 +
drivers/acpi/custom_method.c | 3 +
drivers/acpi/osl.c | 2 +-
drivers/acpi/tables.c | 5 ++
drivers/char/mem.c | 2 +
drivers/input/misc/uinput.c | 1 +
drivers/pci/pci-sysfs.c | 9 +++
drivers/pci/proc.c | 9 ++-
drivers/pci/syscall.c | 3 +-
drivers/pcmcia/cistpl.c | 3 +
drivers/tty/serial/serial_core.c | 6 ++
drivers/tty/sysrq.c | 19 ++++--
fs/debugfs/file.c | 28 +++++++++
fs/debugfs/inode.c | 30 +++++++++-
fs/proc/kcore.c | 2 +
include/linux/input.h | 5 ++
include/linux/kernel.h | 17 ++++++
include/linux/kexec.h | 4 +-
include/linux/security.h | 9 ++-
include/linux/sysrq.h | 8 ++-
kernel/bpf/syscall.c | 3 +
kernel/debug/kdb/kdb_main.c | 2 +-
kernel/events/core.c | 5 ++
kernel/kexec.c | 7 +++
kernel/kexec_file.c | 54 ++++++++++++++---
kernel/kprobes.c | 3 +
kernel/module.c | 39 +++++++++---
kernel/params.c | 26 ++++++--
kernel/power/hibernate.c | 2 +-
kernel/power/user.c | 3 +
security/Kconfig | 24 ++++++++
security/Makefile | 3 +
security/lock_down.c | 106 +++++++++++++++++++++++++++++++++
40 files changed, 447 insertions(+), 44 deletions(-)
create mode 100644 security/lock_down.c




2019-03-07 00:00:01

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 01/27] Add the ability to lock down access to the running kernel image

From: David Howells <[email protected]>

Provide a single call to allow kernel code to determine whether the system
should be locked down, thereby disallowing various accesses that might
allow the running kernel image to be changed including the loading of
modules that aren't validly signed with a key we recognise, fiddling with
MSR registers and disallowing hibernation.

Signed-off-by: David Howells <[email protected]>
Acked-by: James Morris <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
include/linux/kernel.h | 17 ++++++++++++
include/linux/security.h | 9 +++++-
security/Kconfig | 15 ++++++++++
security/Makefile | 3 ++
security/lock_down.c | 59 ++++++++++++++++++++++++++++++++++++++++
5 files changed, 102 insertions(+), 1 deletion(-)
create mode 100644 security/lock_down.c

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 8f0e68e250a7..833bf32ce4e6 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -340,6 +340,23 @@ static inline void refcount_error_report(struct pt_regs *regs, const char *err)
{ }
#endif

+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern bool __kernel_is_locked_down(const char *what, bool first);
+#else
+static inline bool __kernel_is_locked_down(const char *what, bool first)
+{
+ return false;
+}
+#endif
+
+#define kernel_is_locked_down(what) \
+ ({ \
+ static bool message_given; \
+ bool locked_down = __kernel_is_locked_down(what, !message_given); \
+ message_given = true; \
+ locked_down; \
+ })
+
/* Internal, do not use. */
int __must_check _kstrtoul(const char *s, unsigned int base, unsigned long *res);
int __must_check _kstrtol(const char *s, unsigned int base, long *res);
diff --git a/include/linux/security.h b/include/linux/security.h
index 13537a49ae97..b290946341a4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1798,5 +1798,12 @@ static inline void security_bpf_prog_free(struct bpf_prog_aux *aux)
#endif /* CONFIG_SECURITY */
#endif /* CONFIG_BPF_SYSCALL */

-#endif /* ! __LINUX_SECURITY_H */
+#ifdef CONFIG_LOCK_DOWN_KERNEL
+extern void __init init_lockdown(void);
+#else
+static inline void __init init_lockdown(void)
+{
+}
+#endif

+#endif /* ! __LINUX_SECURITY_H */
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..47dc3403b5af 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -229,6 +229,21 @@ config STATIC_USERMODEHELPER_PATH
If you wish for all usermode helper programs to be disabled,
specify an empty string here (i.e. "").

+config LOCK_DOWN_KERNEL
+ bool "Allow the kernel to be 'locked down'"
+ help
+ Allow the kernel to be locked down. If lockdown support is enabled
+ and activated, the kernel will impose additional restrictions
+ intended to prevent uid 0 from being able to modify the running
+ kernel. This may break userland applications that rely on low-level
+ access to hardware.
+
+config LOCK_DOWN_KERNEL_FORCE
+ bool "Enable kernel lockdown mode automatically"
+ depends on LOCK_DOWN_KERNEL
+ help
+ Enable the kernel lock down functionality automatically at boot.
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..5ff090149c88 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -32,3 +32,6 @@ obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
# Object integrity file lists
subdir-$(CONFIG_INTEGRITY) += integrity
obj-$(CONFIG_INTEGRITY) += integrity/
+
+# Allow the kernel to be locked down
+obj-$(CONFIG_LOCK_DOWN_KERNEL) += lock_down.o
diff --git a/security/lock_down.c b/security/lock_down.c
new file mode 100644
index 000000000000..13a8228c1034
--- /dev/null
+++ b/security/lock_down.c
@@ -0,0 +1,59 @@
+/* Lock down the kernel
+ *
+ * Copyright (C) 2016 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells ([email protected])
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/security.h>
+#include <linux/export.h>
+
+static __ro_after_init bool kernel_locked_down;
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static void __init lock_kernel_down(const char *where)
+{
+ if (!kernel_locked_down) {
+ kernel_locked_down = true;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ }
+}
+
+static int __init lockdown_param(char *ignored)
+{
+ lock_kernel_down("command line");
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/*
+ * Lock the kernel down from very early in the arch setup. This must happen
+ * prior to things like ACPI being initialised.
+ */
+void __init init_lockdown(void)
+{
+#ifdef CONFIG_LOCK_DOWN_FORCE
+ lock_kernel_down("Kernel configuration");
+#endif
+}
+
+/**
+ * kernel_is_locked_down - Find out if the kernel is locked down
+ * @what: Tag to use in notice generated if lockdown is in effect
+ */
+bool __kernel_is_locked_down(const char *what, bool first)
+{
+ if (what && first && kernel_locked_down)
+ pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
+ what);
+ return kernel_locked_down;
+}
+EXPORT_SYMBOL(__kernel_is_locked_down);
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:10

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 03/27] Enforce module signatures if the kernel is locked down

From: David Howells <[email protected]>

If the kernel is locked down, require that all modules have valid
signatures that we can verify.

I have adjusted the errors generated:

(1) If there's no signature (ENODATA) or we can't check it (ENOPKG,
ENOKEY), then:

(a) If signatures are enforced then EKEYREJECTED is returned.

(b) If there's no signature or we can't check it, but the kernel is
locked down then EPERM is returned (this is then consistent with
other lockdown cases).

(2) If the signature is unparseable (EBADMSG, EINVAL), the signature fails
the check (EKEYREJECTED) or a system error occurs (eg. ENOMEM), we
return the error we got.

Note that the X.509 code doesn't check for key expiry as the RTC might not
be valid or might not have been transferred to the kernel's clock yet.

[Modified by Matthew Garrett to remove the IMA integration. This will
be replaced with integration with the IMA architecture policy
patchset.]

Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: "Lee, Chun-Yi" <[email protected]>
cc: James Morris <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/module.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 2ad1b5239910..9a377c6ea200 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2767,8 +2767,9 @@ static inline void kmemleak_load_module(const struct module *mod,
#ifdef CONFIG_MODULE_SIG
static int module_sig_check(struct load_info *info, int flags)
{
- int err = -ENOKEY;
+ int err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;

/*
@@ -2783,16 +2784,40 @@ static int module_sig_check(struct load_info *info, int flags)
err = mod_verify_sig(mod, info);
}

- if (!err) {
+ switch (err) {
+ case 0:
info->sig_ok = true;
return 0;
- }

- /* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !is_module_sig_enforced())
- err = 0;
+ /* We don't permit modules to be loaded into trusted kernels
+ * without a valid signature on them, but if we're not
+ * enforcing, certain errors are non-fatal.
+ */
+ case -ENODATA:
+ reason = "Loading of unsigned module";
+ goto decide;
+ case -ENOPKG:
+ reason = "Loading of module with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "Loading of module with unavailable key";
+ decide:
+ if (is_module_sig_enforced()) {
+ pr_notice("%s is rejected\n", reason);
+ return -EKEYREJECTED;
+ }

- return err;
+ if (kernel_is_locked_down(reason))
+ return -EPERM;
+ return 0;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ return err;
+ }
}
#else /* !CONFIG_MODULE_SIG */
static int module_sig_check(struct load_info *info, int flags)
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:17

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 04/27] Restrict /dev/{mem,kmem,port} when the kernel is locked down

From: Matthew Garrett <[email protected]>

Allowing users to read and write to core kernel memory makes it possible
for the kernel to be subverted, avoiding module loading restrictions, and
also to steal cryptographic information.

Disallow /dev/mem and /dev/kmem from being opened this when the kernel has
been locked down to prevent this.

Also disallow /dev/port from being opened to prevent raw ioport access and
thus DMA from being used to accomplish the same thing.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/char/mem.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index b08dc50f9f26..0a2f2e75d5f4 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -786,6 +786,8 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)

static int open_port(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down("/dev/mem,kmem,port"))
+ return -EPERM;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}

--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:24

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 05/27] kexec_load: Disable at runtime if the kernel is locked down

From: Matthew Garrett <[email protected]>

The kexec_load() syscall permits the loading and execution of arbitrary
code in ring 0, which is something that lock-down is meant to prevent. It
makes sense to disable kexec_load() in this situation.

This does not affect kexec_file_load() syscall which can check for a
signature on the image to be booted.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Dave Young <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Reviewed-by: James Morris <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/kexec.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/kernel/kexec.c b/kernel/kexec.c
index 68559808fdfa..8ea0ce31271f 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -207,6 +207,13 @@ static inline int kexec_load_check(unsigned long nr_segments,
if (result < 0)
return result;

+ /*
+ * kexec can be used to circumvent module loading restrictions, so
+ * prevent loading in that case
+ */
+ if (kernel_is_locked_down("kexec of unsigned images"))
+ return -EPERM;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:34

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 06/27] Copy secure_boot flag in boot params across kexec reboot

From: Dave Young <[email protected]>

Kexec reboot in case secure boot being enabled does not keep the secure
boot mode in new kernel, so later one can load unsigned kernel via legacy
kexec_load. In this state, the system is missing the protections provided
by secure boot.

Adding a patch to fix this by retain the secure_boot flag in original
kernel.

secure_boot flag in boot_params is set in EFI stub, but kexec bypasses the
stub. Fixing this issue by copying secure_boot flag across kexec reboot.

Signed-off-by: Dave Young <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/kernel/kexec-bzimage64.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 278cd07228dd..d49554b948fd 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -179,6 +179,7 @@ setup_efi_state(struct boot_params *params, unsigned long params_load_addr,
if (efi_enabled(EFI_OLD_MEMMAP))
return 0;

+ params->secure_boot = boot_params.secure_boot;
ei->efi_loader_signature = current_ei->efi_loader_signature;
ei->efi_systab = current_ei->efi_systab;
ei->efi_systab_hi = current_ei->efi_systab_hi;
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:40

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 07/27] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE

From: Jiri Bohac <[email protected]>

This is a preparatory patch for kexec_file_load() lockdown. A locked down
kernel needs to prevent unsigned kernel images from being loaded with
kexec_file_load(). Currently, the only way to force the signature
verification is compiling with KEXEC_VERIFY_SIG. This prevents loading
usigned images even when the kernel is not locked down at runtime.

This patch splits KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE.
Analogous to the MODULE_SIG and MODULE_SIG_FORCE for modules, KEXEC_SIG
turns on the signature verification but allows unsigned images to be
loaded. KEXEC_SIG_FORCE disallows images without a valid signature.

[Modified by David Howells such that:

(1) verify_pefile_signature() differentiates between no-signature and
sig-didn't-match in its returned errors.

(2) kexec fails with EKEYREJECTED and logs an appropriate message if
signature checking is enforced and an signature is not found, uses
unsupported crypto or has no matching key.

(3) kexec fails with EKEYREJECTED if there is a signature for which we
have a key, but signature doesn't match - even if in non-forcing mode.

(4) kexec fails with EBADMSG or some other error if there is a signature
which cannot be parsed - even if in non-forcing mode.

(5) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
the signature - even if in non-forcing mode.

]

Signed-off-by: Jiri Bohac <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: Matthew Garrett <[email protected]>
cc: Chun-Yi Lee <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/Kconfig | 20 ++++++++---
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c | 48 ++++++++++++++++++++++----
4 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 4b4a7f32b68e..735d04a4b18f 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2016,20 +2016,30 @@ config KEXEC_FILE
config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE

-config KEXEC_VERIFY_SIG
+config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
depends on KEXEC_FILE
---help---
- This option makes kernel signature verification mandatory for
- the kexec_file_load() syscall.

- In addition to that option, you need to enable signature
+ This option makes the kexec_file_load() syscall check for a valid
+ signature of the kernel image. The image can still be loaded without
+ a valid signature unless you also enable KEXEC_SIG_FORCE, though if
+ there's a signature that we can check, then it must be valid.
+
+ In addition to this option, you need to enable signature
verification for the corresponding kernel image type being
loaded in order for this to work.

+config KEXEC_SIG_FORCE
+ bool "Require a valid signature in kexec_file_load() syscall"
+ depends on KEXEC_SIG
+ ---help---
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
config KEXEC_BZIMAGE_VERIFY_SIG
bool "Enable bzImage signature verification support"
- depends on KEXEC_VERIFY_SIG
+ depends on KEXEC_SIG
depends on SIGNED_PE_FILE_VERIFICATION
select SYSTEM_TRUSTED_KEYRING
---help---
diff --git a/crypto/asymmetric_keys/verify_pefile.c b/crypto/asymmetric_keys/verify_pefile.c
index d178650fd524..4473cea1e877 100644
--- a/crypto/asymmetric_keys/verify_pefile.c
+++ b/crypto/asymmetric_keys/verify_pefile.c
@@ -100,7 +100,7 @@ static int pefile_parse_binary(const void *pebuf, unsigned int pelen,

if (!ddir->certs.virtual_address || !ddir->certs.size) {
pr_debug("Unsigned PE binary\n");
- return -EKEYREJECTED;
+ return -ENODATA;
}

chkaddr(ctx->header_size, ddir->certs.virtual_address,
@@ -408,6 +408,8 @@ static int pefile_digest_pe(const void *pebuf, unsigned int pelen,
* (*) 0 if at least one signature chain intersects with the keys in the trust
* keyring, or:
*
+ * (*) -ENODATA if there is no signature present.
+ *
* (*) -ENOPKG if a suitable crypto module couldn't be found for a check on a
* chain.
*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index b9b1bc5f9669..58b27c7bdc2b 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -125,7 +125,7 @@ typedef void *(kexec_load_t)(struct kimage *image, char *kernel_buf,
unsigned long cmdline_len);
typedef int (kexec_cleanup_t)(void *loader_data);

-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
typedef int (kexec_verify_sig_t)(const char *kernel_buf,
unsigned long kernel_len);
#endif
@@ -134,7 +134,7 @@ struct kexec_file_ops {
kexec_probe_t *probe;
kexec_load_t *load;
kexec_cleanup_t *cleanup;
-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
kexec_verify_sig_t *verify_sig;
#endif
};
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index f1d0e00a3971..67f3a866eabe 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -90,7 +90,7 @@ int __weak arch_kimage_file_post_load_cleanup(struct kimage *image)
return kexec_image_post_load_cleanup_default(image);
}

-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
static int kexec_image_verify_sig_default(struct kimage *image, void *buf,
unsigned long buf_len)
{
@@ -188,7 +188,8 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
const char __user *cmdline_ptr,
unsigned long cmdline_len, unsigned flags)
{
- int ret = 0;
+ const char *reason;
+ int ret;
void *ldata;
loff_t size;

@@ -207,15 +208,48 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
if (ret)
goto out;

-#ifdef CONFIG_KEXEC_VERIFY_SIG
+#ifdef CONFIG_KEXEC_SIG
ret = arch_kexec_kernel_verify_sig(image, image->kernel_buf,
image->kernel_buf_len);
- if (ret) {
- pr_debug("kernel signature verification failed.\n");
+#else
+ ret = -ENODATA;
+#endif
+
+ switch (ret) {
+ case 0:
+ break;
+
+ /* Certain verification errors are non-fatal if we're not
+ * checking errors, provided we aren't mandating that there
+ * must be a valid signature.
+ */
+ case -ENODATA:
+ reason = "kexec of unsigned image";
+ goto decide;
+ case -ENOPKG:
+ reason = "kexec of image with unsupported crypto";
+ goto decide;
+ case -ENOKEY:
+ reason = "kexec of image with unavailable key";
+ decide:
+ if (IS_ENABLED(CONFIG_KEXEC_SIG_FORCE)) {
+ pr_notice("%s rejected\n", reason);
+ ret = -EKEYREJECTED;
+ goto out;
+ }
+
+ ret = 0;
+ break;
+
+ /* All other errors are fatal, including nomem, unparseable
+ * signatures and signature check failures - even if signatures
+ * aren't required.
+ */
+ default:
+ pr_notice("kernel signature verification failed (%d).\n", ret);
goto out;
}
- pr_debug("kernel signature verification successful.\n");
-#endif
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, &image->initrd_buf,
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:00:54

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 10/27] uswsusp: Disable when the kernel is locked down

From: Matthew Garrett <[email protected]>

uswsusp allows a user process to dump and then restore kernel state, which
makes it possible to modify the running kernel. Disable this if the kernel
is locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Reviewed-by: James Morris <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/power/user.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/power/user.c b/kernel/power/user.c
index 2d8b60a3c86b..0305d513c274 100644
--- a/kernel/power/user.c
+++ b/kernel/power/user.c
@@ -52,6 +52,9 @@ static int snapshot_open(struct inode *inode, struct file *filp)
if (!hibernation_available())
return -EPERM;

+ if (kernel_is_locked_down("/dev/snapshot"))
+ return -EPERM;
+
lock_system_sleep();

if (!atomic_add_unless(&snapshot_device_available, -1, 0)) {
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:05

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 11/27] PCI: Lock down BAR access when the kernel is locked down

From: Matthew Garrett <[email protected]>

Any hardware that can potentially generate DMA has to be locked down in
order to avoid it being possible for an attacker to modify kernel code,
allowing them to circumvent disabled module loading or module signing.
Default to paranoid - in future we can potentially relax this for
sufficiently IOMMU-isolated devices.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pci/pci-sysfs.c | 9 +++++++++
drivers/pci/proc.c | 9 ++++++++-
drivers/pci/syscall.c | 3 ++-
3 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 9ecfe13157c0..40c14574fcf8 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -905,6 +905,9 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
loff_t init_off = off;
u8 *data = (u8 *) buf;

+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
if (off > dev->cfg_size)
return 0;
if (off + count > dev->cfg_size) {
@@ -1167,6 +1170,9 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
enum pci_mmap_state mmap_type;
struct resource *res = &pdev->resource[bar];

+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;

@@ -1242,6 +1248,9 @@ static ssize_t pci_write_resource_io(struct file *filp, struct kobject *kobj,
struct bin_attribute *attr, char *buf,
loff_t off, size_t count)
{
+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
return pci_resource_io(filp, kobj, attr, buf, off, count, true);
}

diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 6fa1627ce08d..1549cdd0710e 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -117,6 +117,9 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
int size = dev->cfg_size;
int cnt;

+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
if (pos >= size)
return 0;
if (nbytes >= size)
@@ -196,6 +199,9 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
#endif /* HAVE_PCI_MMAP */
int ret = 0;

+ if (kernel_is_locked_down("Direct PCI access"))
+ return -EPERM;
+
switch (cmd) {
case PCIIOC_CONTROLLER:
ret = pci_domain_nr(dev->bus);
@@ -237,7 +243,8 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
struct pci_filp_private *fpriv = file->private_data;
int i, ret, write_combine = 0, res_bit = IORESOURCE_MEM;

- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) ||
+ kernel_is_locked_down("Direct PCI access"))
return -EPERM;

if (fpriv->mmap_state == pci_mmap_io) {
diff --git a/drivers/pci/syscall.c b/drivers/pci/syscall.c
index d96626c614f5..b8a08d3166a1 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -90,7 +90,8 @@ SYSCALL_DEFINE5(pciconfig_write, unsigned long, bus, unsigned long, dfn,
u32 dword;
int err = 0;

- if (!capable(CAP_SYS_ADMIN))
+ if (!capable(CAP_SYS_ADMIN) ||
+ kernel_is_locked_down("Direct PCI access"))
return -EPERM;

dev = pci_get_domain_bus_and_slot(0, bus, dfn);
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:18

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 17/27] acpi: Disable APEI error injection if the kernel is locked down

From: Linn Crosetto <[email protected]>

ACPI provides an error injection mechanism, EINJ, for debugging and testing
the ACPI Platform Error Interface (APEI) and other RAS features. If
supported by the firmware, ACPI specification 5.0 and later provide for a
way to specify a physical memory address to which to inject the error.

Injecting errors through EINJ can produce errors which to the platform are
indistinguishable from real hardware errors. This can have undesirable
side-effects, such as causing the platform to mark hardware as needing
replacement.

While it does not provide a method to load unauthenticated privileged code,
the effect of these errors may persist across reboots and affect trust in
the underlying hardware, so disable error injection through EINJ if
the kernel is locked down.

Signed-off-by: Linn Crosetto <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/apei/einj.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c
index fcccbfdbdd1a..9fe6bbab2e7d 100644
--- a/drivers/acpi/apei/einj.c
+++ b/drivers/acpi/apei/einj.c
@@ -518,6 +518,9 @@ static int einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2,
int rc;
u64 base_addr, size;

+ if (kernel_is_locked_down("ACPI error injection"))
+ return -EPERM;
+
/* If user manually set "flags", make sure it is legal */
if (flags && (flags &
~(SETWA_FLAGS_APICID|SETWA_FLAGS_MEM|SETWA_FLAGS_PCIE_SBDF)))
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:33

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 26/27] debugfs: Restrict debugfs when the kernel is locked down

From: David Howells <[email protected]>

Disallow opening of debugfs files that might be used to muck around when
the kernel is locked down as various drivers give raw access to hardware
through debugfs. Given the effort of auditing all 2000 or so files and
manually fixing each one as necessary, I've chosen to apply a heuristic
instead. The following changes are made:

(1) chmod and chown are disallowed on debugfs objects (though the root dir
can be modified by mount and remount, but I'm not worried about that).

(2) When the kernel is locked down, only files with the following criteria
are permitted to be opened:

- The file must have mode 00444
- The file must not have ioctl methods
- The file must not have mmap

(3) When the kernel is locked down, files may only be opened for reading.

Normal device interaction should be done through configfs, sysfs or a
miscdev, not debugfs.

Note that this makes it unnecessary to specifically lock down show_dsts(),
show_devs() and show_call() in the asus-wmi driver.

I would actually prefer to lock down all files by default and have the
the files unlocked by the creator. This is tricky to manage correctly,
though, as there are 19 creation functions and ~1600 call sites (some of
them in loops scanning tables).

Signed-off-by: David Howells <[email protected]>
cc: Andy Shevchenko <[email protected]>
cc: [email protected]
cc: [email protected]
cc: Matthew Garrett <[email protected]>
cc: Thomas Gleixner <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
fs/debugfs/file.c | 28 ++++++++++++++++++++++++++++
fs/debugfs/inode.c | 30 ++++++++++++++++++++++++++++--
2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 4fce1da7db23..c33042c1eff3 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -136,6 +136,25 @@ void debugfs_file_put(struct dentry *dentry)
}
EXPORT_SYMBOL_GPL(debugfs_file_put);

+/*
+ * Only permit access to world-readable files when the kernel is locked down.
+ * We also need to exclude any file that has ways to write or alter it as root
+ * can bypass the permissions check.
+ */
+static bool debugfs_is_locked_down(struct inode *inode,
+ struct file *filp,
+ const struct file_operations *real_fops)
+{
+ if ((inode->i_mode & 07777) == 0444 &&
+ !(filp->f_mode & FMODE_WRITE) &&
+ !real_fops->unlocked_ioctl &&
+ !real_fops->compat_ioctl &&
+ !real_fops->mmap)
+ return false;
+
+ return kernel_is_locked_down("debugfs");
+}
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
struct dentry *dentry = F_DENTRY(filp);
@@ -147,6 +166,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;

real_fops = debugfs_real_fops(filp);
+
+ r = -EPERM;
+ if (debugfs_is_locked_down(inode, filp, real_fops))
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not clean up after itself at exit? */
@@ -272,6 +296,10 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;

real_fops = debugfs_real_fops(filp);
+ r = -EPERM;
+ if (debugfs_is_locked_down(inode, filp, real_fops))
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not cleanup after itself at exit? */
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 13b01351dd1c..4daec17b8215 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -32,6 +32,31 @@ static struct vfsmount *debugfs_mount;
static int debugfs_mount_count;
static bool debugfs_registered;

+/*
+ * Don't allow access attributes to be changed whilst the kernel is locked down
+ * so that we can use the file mode as part of a heuristic to determine whether
+ * to lock down individual files.
+ */
+static int debugfs_setattr(struct dentry *dentry, struct iattr *ia)
+{
+ if ((ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)) &&
+ kernel_is_locked_down("debugfs"))
+ return -EPERM;
+ return simple_setattr(dentry, ia);
+}
+
+static const struct inode_operations debugfs_file_inode_operations = {
+ .setattr = debugfs_setattr,
+};
+static const struct inode_operations debugfs_dir_inode_operations = {
+ .lookup = simple_lookup,
+ .setattr = debugfs_setattr,
+};
+static const struct inode_operations debugfs_symlink_inode_operations = {
+ .get_link = simple_get_link,
+ .setattr = debugfs_setattr,
+};
+
static struct inode *debugfs_get_inode(struct super_block *sb)
{
struct inode *inode = new_inode(sb);
@@ -356,6 +381,7 @@ static struct dentry *__debugfs_create_file(const char *name, umode_t mode,
inode->i_mode = mode;
inode->i_private = data;

+ inode->i_op = &debugfs_file_inode_operations;
inode->i_fop = proxy_fops;
dentry->d_fsdata = (void *)((unsigned long)real_fops |
DEBUGFS_FSDATA_IS_REAL_FOPS_BIT);
@@ -513,7 +539,7 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent)
return failed_creating(dentry);

inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO;
- inode->i_op = &simple_dir_inode_operations;
+ inode->i_op = &debugfs_dir_inode_operations;
inode->i_fop = &simple_dir_operations;

/* directory inodes start off with i_nlink == 2 (for "." entry) */
@@ -608,7 +634,7 @@ struct dentry *debugfs_create_symlink(const char *name, struct dentry *parent,
return failed_creating(dentry);
}
inode->i_mode = S_IFLNK | S_IRWXUGO;
- inode->i_op = &simple_symlink_inode_operations;
+ inode->i_op = &debugfs_symlink_inode_operations;
inode->i_link = link;
d_instantiate(dentry, inode);
return end_creating(dentry);
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:33

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 25/27] Lock down perf

From: David Howells <[email protected]>

Disallow the use of certain perf facilities that might allow userspace to
access kernel data.

Signed-off-by: David Howells <[email protected]>

Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/events/core.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 3cd13a30f732..7748c6f39992 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10461,6 +10461,11 @@ SYSCALL_DEFINE5(perf_event_open,
return -EINVAL;
}

+ if ((attr.sample_type & PERF_SAMPLE_REGS_INTR) &&
+ kernel_is_locked_down("PERF_SAMPLE_REGS_INTR"))
+ /* REGS_INTR can leak data, lockdown must prevent this */
+ return -EPERM;
+
/* Only privileged users can get physical addresses */
if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:34

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 14/27] ACPI: Limit access to custom_method when the kernel is locked down

From: Matthew Garrett <[email protected]>

custom_method effectively allows arbitrary access to system memory, making
it possible for an attacker to circumvent restrictions on module loading.
Disable it if the kernel is locked down.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/custom_method.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index 4451877f83b6..ac8a90dc7096 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -29,6 +29,9 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
struct acpi_table_header table;
acpi_status status;

+ if (kernel_is_locked_down("ACPI custom methods"))
+ return -EPERM;
+
if (!(*ppos)) {
/* parse the table header to get the table length */
if (count <= sizeof(struct acpi_table_header))
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:41

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 27/27] lockdown: Print current->comm in restriction messages

From: David Howells <[email protected]>

Print the content of current->comm in messages generated by lockdown to
indicate a restriction that was hit. This makes it a bit easier to find
out what caused the message.

The message now patterned something like:

Lockdown: <comm>: <what> is restricted; see man kernel_lockdown.7

Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
security/lock_down.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/lock_down.c b/security/lock_down.c
index cfbc2c39712b..5243b55b3c1f 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -58,8 +58,8 @@ void __init init_lockdown(void)
bool __kernel_is_locked_down(const char *what, bool first)
{
if (what && first && kernel_locked_down)
- pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
- what);
+ pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
+ current->comm, what);
return kernel_locked_down;
}
EXPORT_SYMBOL(__kernel_is_locked_down);
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:01:59

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 21/27] x86/mmiotrace: Lock down the testmmiotrace module

From: David Howells <[email protected]>

The testmmiotrace module shouldn't be permitted when the kernel is locked
down as it can be used to arbitrarily read and write MMIO space.

Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: David Howells <[email protected]
cc: Thomas Gleixner <[email protected]>
cc: Steven Rostedt <[email protected]>
cc: Ingo Molnar <[email protected]>
cc: "H. Peter Anvin" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/mm/testmmiotrace.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
index f6ae6830b341..bbaad357f5d7 100644
--- a/arch/x86/mm/testmmiotrace.c
+++ b/arch/x86/mm/testmmiotrace.c
@@ -115,6 +115,9 @@ static int __init init(void)
{
unsigned long size = (read_far) ? (8 << 20) : (16 << 10);

+ if (kernel_is_locked_down("MMIO trace testing"))
+ return -EPERM;
+
if (mmio_address == 0) {
pr_err("you have to use the module argument mmio_address.\n");
pr_err("DO NOT LOAD THIS MODULE UNLESS YOU REALLY KNOW WHAT YOU ARE DOING!\n");
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:10

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 18/27] Prohibit PCMCIA CIS storage when the kernel is locked down

From: David Howells <[email protected]>

Prohibit replacement of the PCMCIA Card Information Structure when the
kernel is locked down.

Suggested-by: Dominik Brodowski <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pcmcia/cistpl.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index ac0672b8dfca..8adf092d0e18 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -1578,6 +1578,9 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
struct pcmcia_socket *s;
int error;

+ if (kernel_is_locked_down("Direct PCMCIA CIS storage"))
+ return -EPERM;
+
s = to_socket(container_of(kobj, struct device, kobj));

if (off)
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:18

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 16/27] acpi: Disable ACPI table override if the kernel is locked down

From: Linn Crosetto <[email protected]>

From the kernel documentation (initrd_table_override.txt):

If the ACPI_INITRD_TABLE_OVERRIDE compile option is true, it is possible
to override nearly any ACPI table provided by the BIOS with an
instrumented, modified one.

When securelevel is set, the kernel should disallow any unauthenticated
changes to kernel space. ACPI tables contain code invoked by the kernel,
so do not allow ACPI tables to be overridden if the kernel is locked down.

Signed-off-by: Linn Crosetto <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/tables.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 48eabb6c2d4f..f3b4117cd8f3 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -531,6 +531,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;

+ if (kernel_is_locked_down("ACPI table override")) {
+ pr_notice("kernel is locked down, ignoring table override\n");
+ return;
+ }
+
acpi_tables_addr =
memblock_find_in_range(0, ACPI_TABLE_UPGRADE_MAX_PHYS,
all_tables_size, PAGE_SIZE);
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:20

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 13/27] x86/msr: Restrict MSR access when the kernel is locked down

From: Matthew Garrett <[email protected]>

Writing to MSRs should not be allowed if the kernel is locked down, since
it could lead to execution of arbitrary code in kernel mode. Based on a
patch by Kees Cook.

MSR accesses are logged for the purposes of building up a whitelist as per
Alan Cox's suggestion.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Acked-by: Kees Cook <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/kernel/msr.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 4588414e2561..f5a2cf07972f 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -84,6 +84,11 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;

+ if (kernel_is_locked_down("Direct MSR access")) {
+ pr_info("Direct access to MSR %x\n", reg);
+ return -EPERM;
+ }
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */

@@ -135,6 +140,11 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
err = -EFAULT;
break;
}
+ if (kernel_is_locked_down("Direct MSR access")) {
+ pr_info("Direct access to MSR %x\n", regs[1]); /* Display %ecx */
+ err = -EPERM;
+ break;
+ }
err = wrmsr_safe_regs_on_cpu(cpu, regs);
if (err)
break;
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:26

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 20/27] Lock down module params that specify hardware parameters (eg. ioport)

From: David Howells <[email protected]>

Provided an annotation for module parameters that specify hardware
parameters (such as io ports, iomem addresses, irqs, dma channels, fixed
dma buffers and other types).

Suggested-by: Alan Cox <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/params.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/kernel/params.c b/kernel/params.c
index ce89f757e6da..8ac751c938f8 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -108,13 +108,19 @@ bool parameq(const char *a, const char *b)
return parameqn(a, b, strlen(a)+1);
}

-static void param_check_unsafe(const struct kernel_param *kp)
+static bool param_check_unsafe(const struct kernel_param *kp,
+ const char *doing)
{
if (kp->flags & KERNEL_PARAM_FL_UNSAFE) {
pr_notice("Setting dangerous option %s - tainting kernel\n",
kp->name);
add_taint(TAINT_USER, LOCKDEP_STILL_OK);
}
+
+ if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
+ kernel_is_locked_down("Command line-specified device addresses, irqs and dma channels"))
+ return false;
+ return true;
}

static int parse_one(char *param,
@@ -144,8 +150,10 @@ static int parse_one(char *param,
pr_debug("handling %s with %p\n", param,
params[i].ops->set);
kernel_param_lock(params[i].mod);
- param_check_unsafe(&params[i]);
- err = params[i].ops->set(val, &params[i]);
+ if (param_check_unsafe(&params[i], doing))
+ err = params[i].ops->set(val, &params[i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -553,6 +561,12 @@ static ssize_t param_attr_show(struct module_attribute *mattr,
return count;
}

+#ifdef CONFIG_MODULES
+#define mod_name(mod) (mod)->name
+#else
+#define mod_name(mod) "unknown"
+#endif
+
/* sysfs always hands a nul-terminated string in buf. We rely on that. */
static ssize_t param_attr_store(struct module_attribute *mattr,
struct module_kobject *mk,
@@ -565,8 +579,10 @@ static ssize_t param_attr_store(struct module_attribute *mattr,
return -EPERM;

kernel_param_lock(mk->mod);
- param_check_unsafe(attribute->param);
- err = attribute->param->ops->set(buf, attribute->param);
+ if (param_check_unsafe(attribute->param, mod_name(mk->mod)))
+ err = attribute->param->ops->set(buf, attribute->param);
+ else
+ err = -EPERM;
kernel_param_unlock(mk->mod);
if (!err)
return len;
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:28

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 23/27] Lock down kprobes

From: David Howells <[email protected]>

Disallow the creation of kprobes when the kernel is locked down by
preventing their registration. This prevents kprobes from being used to
access kernel memory, either to make modifications or to steal crypto data.

Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/kprobes.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f4ddfdd2d07e..6f66cca8e2c6 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1552,6 +1552,9 @@ int register_kprobe(struct kprobe *p)
struct module *probed_mod;
kprobe_opcode_t *addr;

+ if (kernel_is_locked_down("Use of kprobes"))
+ return -EPERM;
+
/* Adjust probe address from symbol */
addr = kprobe_addr(p);
if (IS_ERR(addr))
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:40

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 22/27] Lock down /proc/kcore

From: David Howells <[email protected]>

Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data.

Signed-off-by: David Howells <[email protected]>
Reviewed-by: James Morris <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
fs/proc/kcore.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index bbcc185062bb..d50ebfbf3dbb 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -518,6 +518,8 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)

static int open_kcore(struct inode *inode, struct file *filp)
{
+ if (kernel_is_locked_down("/proc/kcore"))
+ return -EPERM;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;

--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:02:41

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 24/27] bpf: Restrict kernel image access functions when the kernel is locked down

From: David Howells <[email protected]>

There are some bpf functions can be used to read kernel memory:
bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
private keys in kernel memory (e.g. the hibernation image signing key) to
be read by an eBPF program and kernel memory to be altered without
restriction.

Completely prohibit the use of BPF when the kernel is locked down.

Suggested-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
cc: Chun-Yi Lee <[email protected]>
cc: Alexei Starovoitov <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/bpf/syscall.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index b155cd17c1bd..2cde39a875aa 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2585,6 +2585,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
return -EPERM;

+ if (kernel_is_locked_down("BPF"))
+ return -EPERM;
+
err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
if (err)
return err;
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:03:14

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 19/27] Lock down TIOCSSERIAL

From: David Howells <[email protected]>

Lock down TIOCSSERIAL as that can be used to change the ioport and irq
settings on a serial port. This only appears to be an issue for the serial
drivers that use the core serial code. All other drivers seem to either
ignore attempts to change port/irq or give an error.

Reported-by: Greg Kroah-Hartman <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Jiri Slaby <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/tty/serial/serial_core.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index d4cca5bdaf1c..04534877b575 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -842,6 +842,12 @@ static int uart_set_info(struct tty_struct *tty, struct tty_port *port,
new_flags = (__force upf_t)new_info->flags;
old_custom_divisor = uport->custom_divisor;

+ if ((change_port || change_irq) &&
+ kernel_is_locked_down("Using TIOCSSERIAL to change device addresses, irqs and dma channels")) {
+ retval = -EPERM;
+ goto exit;
+ }
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:03:13

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

From: Kyle McMartin <[email protected]>

Make an option to provide a sysrq key that will lift the kernel lockdown,
thereby allowing the running kernel image to be accessed and modified.

On x86 this is triggered with SysRq+x, but this key may not be available on
all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
Since this macro must be defined in an arch to be able to use this facility
for that arch, the Kconfig option is restricted to arches that support it.

Signed-off-by: Kyle McMartin <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/include/asm/setup.h | 2 ++
drivers/input/misc/uinput.c | 1 +
drivers/tty/sysrq.c | 19 ++++++++++-----
include/linux/input.h | 5 ++++
include/linux/sysrq.h | 8 +++++-
kernel/debug/kdb/kdb_main.c | 2 +-
security/Kconfig | 9 +++++++
security/lock_down.c | 47 ++++++++++++++++++++++++++++++++++++
8 files changed, 85 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
index ed8ec011a9fd..8daf633a5347 100644
--- a/arch/x86/include/asm/setup.h
+++ b/arch/x86/include/asm/setup.h
@@ -9,6 +9,8 @@
#include <linux/linkage.h>
#include <asm/page_types.h>

+#define LOCKDOWN_LIFT_KEY 'x'
+
#ifdef __i386__

#include <linux/pfn.h>
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index 8ec483e8688b..c2a77dc73fa0 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -365,6 +365,7 @@ static int uinput_create_device(struct uinput_device *udev)
dev->flush = uinput_dev_flush;
}

+ dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
dev->event = uinput_dev_event;

input_set_drvdata(udev->dev, udev);
diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 1f03078ec352..0a05d336008e 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -480,6 +480,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
/* x: May be registered on mips for TLB dump */
/* x: May be registered on ppc/powerpc for xmon */
/* x: May be registered on sparc64 for global PMU dump */
+ /* x: May be registered on x86_64 for disabling secure boot */
NULL, /* x */
/* y: May be registered on sparc64 for global register dump */
NULL, /* y */
@@ -523,7 +524,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
sysrq_key_table[i] = op_p;
}

-void __handle_sysrq(int key, bool check_mask)
+void __handle_sysrq(int key, unsigned int from)
{
struct sysrq_key_op *op_p;
int orig_log_level;
@@ -543,11 +544,15 @@ void __handle_sysrq(int key, bool check_mask)

op_p = __sysrq_get_key_op(key);
if (op_p) {
+ /* Ban synthetic events from some sysrq functionality */
+ if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
+ op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
+ printk("This sysrq operation is disabled from userspace.\n");
/*
* Should we check for enabled operations (/proc/sysrq-trigger
* should not) and is the invoked operation enabled?
*/
- if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
+ if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
pr_cont("%s\n", op_p->action_msg);
console_loglevel = orig_log_level;
op_p->handler(key);
@@ -579,7 +584,7 @@ void __handle_sysrq(int key, bool check_mask)
void handle_sysrq(int key)
{
if (sysrq_on())
- __handle_sysrq(key, true);
+ __handle_sysrq(key, SYSRQ_FROM_KERNEL);
}
EXPORT_SYMBOL(handle_sysrq);

@@ -659,7 +664,7 @@ static void sysrq_do_reset(struct timer_list *t)
static void sysrq_handle_reset_request(struct sysrq_state *state)
{
if (state->reset_requested)
- __handle_sysrq(sysrq_xlate[KEY_B], false);
+ __handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);

if (sysrq_reset_downtime_ms)
mod_timer(&state->keyreset_timer,
@@ -812,8 +817,10 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,

default:
if (sysrq->active && value && value != 2) {
+ int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
+ SYSRQ_FROM_SYNTHETIC : 0;
sysrq->need_reinject = false;
- __handle_sysrq(sysrq_xlate[code], true);
+ __handle_sysrq(sysrq_xlate[code], from);
}
break;
}
@@ -1096,7 +1103,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,

if (get_user(c, buf))
return -EFAULT;
- __handle_sysrq(c, false);
+ __handle_sysrq(c, SYSRQ_FROM_PROC);
}

return count;
diff --git a/include/linux/input.h b/include/linux/input.h
index 7c7516eb7d76..38cd0ea72c37 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -42,6 +42,7 @@ struct input_value {
* @phys: physical path to the device in the system hierarchy
* @uniq: unique identification code for the device (if device has it)
* @id: id of the device (struct input_id)
+ * @flags: input device flags (SYNTHETIC, etc.)
* @propbit: bitmap of device properties and quirks
* @evbit: bitmap of types of events supported by the device (EV_KEY,
* EV_REL, etc.)
@@ -124,6 +125,8 @@ struct input_dev {
const char *uniq;
struct input_id id;

+ unsigned int flags;
+
unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];

unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
@@ -190,6 +193,8 @@ struct input_dev {
};
#define to_input_dev(d) container_of(d, struct input_dev, dev)

+#define INPUTDEV_FLAGS_SYNTHETIC 0x000000001
+
/*
* Verify that we are in sync with input_device_id mod_devicetable.h #defines
*/
diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
index 8c71874e8485..7de1f08b60a9 100644
--- a/include/linux/sysrq.h
+++ b/include/linux/sysrq.h
@@ -29,6 +29,8 @@
#define SYSRQ_ENABLE_BOOT 0x0080
#define SYSRQ_ENABLE_RTNICE 0x0100

+#define SYSRQ_DISABLE_USERSPACE 0x00010000
+
struct sysrq_key_op {
void (*handler)(int);
char *help_msg;
@@ -43,8 +45,12 @@ struct sysrq_key_op {
* are available -- else NULL's).
*/

+#define SYSRQ_FROM_KERNEL 0x0001
+#define SYSRQ_FROM_PROC 0x0002
+#define SYSRQ_FROM_SYNTHETIC 0x0004
+
void handle_sysrq(int key);
-void __handle_sysrq(int key, bool check_mask);
+void __handle_sysrq(int key, unsigned int from);
int register_sysrq_key(int key, struct sysrq_key_op *op);
int unregister_sysrq_key(int key, struct sysrq_key_op *op);
struct sysrq_key_op *__sysrq_get_key_op(int key);
diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
index 82a3b32a7cfc..efee1abf5e8e 100644
--- a/kernel/debug/kdb/kdb_main.c
+++ b/kernel/debug/kdb/kdb_main.c
@@ -1981,7 +1981,7 @@ static int kdb_sr(int argc, const char **argv)
return KDB_ARGCOUNT;

kdb_trap_printk++;
- __handle_sysrq(*argv[1], check_mask);
+ __handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
kdb_trap_printk--;

return 0;
diff --git a/security/Kconfig b/security/Kconfig
index 47dc3403b5af..8346eb883336 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -244,6 +244,15 @@ config LOCK_DOWN_KERNEL_FORCE
help
Enable the kernel lock down functionality automatically at boot.

+config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+ bool "Allow the kernel lockdown to be lifted by SysRq"
+ depends on LOCK_DOWN_KERNEL
+ depends on MAGIC_SYSRQ
+ depends on X86
+ help
+ Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
+ combination on a wired keyboard.
+
source "security/selinux/Kconfig"
source "security/smack/Kconfig"
source "security/tomoyo/Kconfig"
diff --git a/security/lock_down.c b/security/lock_down.c
index 13a8228c1034..cfbc2c39712b 100644
--- a/security/lock_down.c
+++ b/security/lock_down.c
@@ -11,8 +11,14 @@

#include <linux/security.h>
#include <linux/export.h>
+#include <linux/sysrq.h>
+#include <asm/setup.h>

+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+static __read_mostly bool kernel_locked_down;
+#else
static __ro_after_init bool kernel_locked_down;
+#endif

/*
* Put the kernel into lock-down mode.
@@ -57,3 +63,44 @@ bool __kernel_is_locked_down(const char *what, bool first)
return kernel_locked_down;
}
EXPORT_SYMBOL(__kernel_is_locked_down);
+
+#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
+
+/*
+ * Take the kernel out of lockdown mode.
+ */
+static void lift_kernel_lockdown(void)
+{
+ pr_notice("Lifting lockdown\n");
+ kernel_locked_down = false;
+}
+
+/*
+ * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
+ * echoing the appropriate letter into the sysrq-trigger file).
+ */
+static void sysrq_handle_lockdown_lift(int key)
+{
+ if (kernel_locked_down)
+ lift_kernel_lockdown();
+}
+
+static struct sysrq_key_op lockdown_lift_sysrq_op = {
+ .handler = sysrq_handle_lockdown_lift,
+ .help_msg = "unSB(x)",
+ .action_msg = "Disabling Secure Boot restrictions",
+ .enable_mask = SYSRQ_DISABLE_USERSPACE,
+};
+
+static int __init lockdown_lift_sysrq(void)
+{
+ if (kernel_locked_down) {
+ lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
+ register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
+ }
+ return 0;
+}
+
+late_initcall(lockdown_lift_sysrq);
+
+#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:03:18

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 09/27] hibernate: Disable when the kernel is locked down

From: Josh Boyer <[email protected]>

There is currently no way to verify the resume image when returning
from hibernate. This might compromise the signed modules trust model,
so until we can work with signed hibernate images we disable it when the
kernel is locked down.

Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/power/hibernate.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index abef759de7c8..802795becb88 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -70,7 +70,7 @@ static const struct platform_hibernation_ops *hibernation_ops;

bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !kernel_is_locked_down("Hibernation");
}

/**
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:03:36

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 15/27] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down

From: Josh Boyer <[email protected]>

This option allows userspace to pass the RSDP address to the kernel, which
makes it possible for a user to modify the workings of hardware . Reject
the option when the kernel is locked down.

Signed-off-by: Josh Boyer <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: Dave Young <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/acpi/osl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f29e427d0d1d..3e44cef7a0cd 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -194,7 +194,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
acpi_physical_address pa;

#ifdef CONFIG_KEXEC
- if (acpi_rsdp)
+ if (acpi_rsdp && !kernel_is_locked_down("ACPI RSDP specification"))
return acpi_rsdp;
#endif
pa = acpi_arch_get_root_pointer();
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:03:38

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 08/27] kexec_file: Restrict at runtime if the kernel is locked down

From: Jiri Bohac <[email protected]>

When KEXEC_SIG is not enabled, kernel should not load images through
kexec_file systemcall if the kernel is locked down.

[Modified by David Howells to fit with modifications to the previous patch
and to return -EPERM if the kernel is locked down for consistency with
other lockdowns. Modified by Matthew Garrett to remove the IMA
integration, which will be replaced by integrating with the IMA
architecture policy patches.]

Signed-off-by: Jiri Bohac <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
Cc: Matthew Garrett <[email protected]>
cc: Chun-Yi Lee <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
kernel/kexec_file.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 67f3a866eabe..0cfe4f6f7f85 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -239,6 +239,12 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
}

ret = 0;
+
+ if (kernel_is_locked_down(reason)) {
+ ret = -EPERM;
+ goto out;
+ }
+
break;

/* All other errors are fatal, including nomem, unparseable
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:04:04

by Matthew Garrett

[permalink] [raw]
Subject: [PATCH 12/27] x86: Lock down IO port access when the kernel is locked down

From: Matthew Garrett <[email protected]>

IO port access would permit users to gain access to PCI configuration
registers, which in turn (on a lot of hardware) give access to MMIO
register space. This would potentially permit root to trigger arbitrary
DMA, so lock it down by default.

This also implicitly locks down the KDADDIO, KDDELIO, KDENABIO and
KDDISABIO console ioctls.

Signed-off-by: Matthew Garrett <[email protected]>
Signed-off-by: David Howells <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
cc: [email protected]
Signed-off-by: Matthew Garrett <[email protected]>
---
arch/x86/kernel/ioport.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 0fe1c8782208..abc702a6ae9c 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -31,7 +31,8 @@ long ksys_ioperm(unsigned long from, unsigned long num, int turn_on)

if ((from + num <= from) || (from + num > IO_BITMAP_BITS))
return -EINVAL;
- if (turn_on && !capable(CAP_SYS_RAWIO))
+ if (turn_on && (!capable(CAP_SYS_RAWIO) ||
+ kernel_is_locked_down("ioperm")))
return -EPERM;

/*
@@ -126,7 +127,8 @@ SYSCALL_DEFINE1(iopl, unsigned int, level)
return -EINVAL;
/* Trying to gain more privileges? */
if (level > old) {
- if (!capable(CAP_SYS_RAWIO))
+ if (!capable(CAP_SYS_RAWIO) ||
+ kernel_is_locked_down("iopl"))
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
--
2.21.0.352.gf09ad66450-goog


2019-03-07 00:10:53

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

On 3/6/19 3:58 PM, Matthew Garrett wrote:
> From: Kyle McMartin <[email protected]>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.

You still need to document this in Documentation/admin-guide/sysrq.rst,
like I mentioned last week.

> On x86 this is triggered with SysRq+x, but this key may not be available on
> all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
> Since this macro must be defined in an arch to be able to use this facility
> for that arch, the Kconfig option is restricted to arches that support it.
>
> Signed-off-by: Kyle McMartin <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> arch/x86/include/asm/setup.h | 2 ++
> drivers/input/misc/uinput.c | 1 +
> drivers/tty/sysrq.c | 19 ++++++++++-----
> include/linux/input.h | 5 ++++
> include/linux/sysrq.h | 8 +++++-
> kernel/debug/kdb/kdb_main.c | 2 +-
> security/Kconfig | 9 +++++++
> security/lock_down.c | 47 ++++++++++++++++++++++++++++++++++++
> 8 files changed, 85 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/setup.h b/arch/x86/include/asm/setup.h
> index ed8ec011a9fd..8daf633a5347 100644
> --- a/arch/x86/include/asm/setup.h
> +++ b/arch/x86/include/asm/setup.h
> @@ -9,6 +9,8 @@
> #include <linux/linkage.h>
> #include <asm/page_types.h>
>
> +#define LOCKDOWN_LIFT_KEY 'x'
> +
> #ifdef __i386__
>
> #include <linux/pfn.h>
> diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
> index 8ec483e8688b..c2a77dc73fa0 100644
> --- a/drivers/input/misc/uinput.c
> +++ b/drivers/input/misc/uinput.c
> @@ -365,6 +365,7 @@ static int uinput_create_device(struct uinput_device *udev)
> dev->flush = uinput_dev_flush;
> }
>
> + dev->flags |= INPUTDEV_FLAGS_SYNTHETIC;
> dev->event = uinput_dev_event;
>
> input_set_drvdata(udev->dev, udev);
> diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
> index 1f03078ec352..0a05d336008e 100644
> --- a/drivers/tty/sysrq.c
> +++ b/drivers/tty/sysrq.c
> @@ -480,6 +480,7 @@ static struct sysrq_key_op *sysrq_key_table[36] = {
> /* x: May be registered on mips for TLB dump */
> /* x: May be registered on ppc/powerpc for xmon */
> /* x: May be registered on sparc64 for global PMU dump */
> + /* x: May be registered on x86_64 for disabling secure boot */
> NULL, /* x */
> /* y: May be registered on sparc64 for global register dump */
> NULL, /* y */
> @@ -523,7 +524,7 @@ static void __sysrq_put_key_op(int key, struct sysrq_key_op *op_p)
> sysrq_key_table[i] = op_p;
> }
>
> -void __handle_sysrq(int key, bool check_mask)
> +void __handle_sysrq(int key, unsigned int from)
> {
> struct sysrq_key_op *op_p;
> int orig_log_level;
> @@ -543,11 +544,15 @@ void __handle_sysrq(int key, bool check_mask)
>
> op_p = __sysrq_get_key_op(key);
> if (op_p) {
> + /* Ban synthetic events from some sysrq functionality */
> + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
> + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
> + printk("This sysrq operation is disabled from userspace.\n");
> /*
> * Should we check for enabled operations (/proc/sysrq-trigger
> * should not) and is the invoked operation enabled?
> */
> - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
> + if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {
> pr_cont("%s\n", op_p->action_msg);
> console_loglevel = orig_log_level;
> op_p->handler(key);
> @@ -579,7 +584,7 @@ void __handle_sysrq(int key, bool check_mask)
> void handle_sysrq(int key)
> {
> if (sysrq_on())
> - __handle_sysrq(key, true);
> + __handle_sysrq(key, SYSRQ_FROM_KERNEL);
> }
> EXPORT_SYMBOL(handle_sysrq);
>
> @@ -659,7 +664,7 @@ static void sysrq_do_reset(struct timer_list *t)
> static void sysrq_handle_reset_request(struct sysrq_state *state)
> {
> if (state->reset_requested)
> - __handle_sysrq(sysrq_xlate[KEY_B], false);
> + __handle_sysrq(sysrq_xlate[KEY_B], SYSRQ_FROM_KERNEL);
>
> if (sysrq_reset_downtime_ms)
> mod_timer(&state->keyreset_timer,
> @@ -812,8 +817,10 @@ static bool sysrq_handle_keypress(struct sysrq_state *sysrq,
>
> default:
> if (sysrq->active && value && value != 2) {
> + int from = sysrq->handle.dev->flags & INPUTDEV_FLAGS_SYNTHETIC ?
> + SYSRQ_FROM_SYNTHETIC : 0;
> sysrq->need_reinject = false;
> - __handle_sysrq(sysrq_xlate[code], true);
> + __handle_sysrq(sysrq_xlate[code], from);
> }
> break;
> }
> @@ -1096,7 +1103,7 @@ static ssize_t write_sysrq_trigger(struct file *file, const char __user *buf,
>
> if (get_user(c, buf))
> return -EFAULT;
> - __handle_sysrq(c, false);
> + __handle_sysrq(c, SYSRQ_FROM_PROC);
> }
>
> return count;
> diff --git a/include/linux/input.h b/include/linux/input.h
> index 7c7516eb7d76..38cd0ea72c37 100644
> --- a/include/linux/input.h
> +++ b/include/linux/input.h
> @@ -42,6 +42,7 @@ struct input_value {
> * @phys: physical path to the device in the system hierarchy
> * @uniq: unique identification code for the device (if device has it)
> * @id: id of the device (struct input_id)
> + * @flags: input device flags (SYNTHETIC, etc.)
> * @propbit: bitmap of device properties and quirks
> * @evbit: bitmap of types of events supported by the device (EV_KEY,
> * EV_REL, etc.)
> @@ -124,6 +125,8 @@ struct input_dev {
> const char *uniq;
> struct input_id id;
>
> + unsigned int flags;
> +
> unsigned long propbit[BITS_TO_LONGS(INPUT_PROP_CNT)];
>
> unsigned long evbit[BITS_TO_LONGS(EV_CNT)];
> @@ -190,6 +193,8 @@ struct input_dev {
> };
> #define to_input_dev(d) container_of(d, struct input_dev, dev)
>
> +#define INPUTDEV_FLAGS_SYNTHETIC 0x000000001
> +
> /*
> * Verify that we are in sync with input_device_id mod_devicetable.h #defines
> */
> diff --git a/include/linux/sysrq.h b/include/linux/sysrq.h
> index 8c71874e8485..7de1f08b60a9 100644
> --- a/include/linux/sysrq.h
> +++ b/include/linux/sysrq.h
> @@ -29,6 +29,8 @@
> #define SYSRQ_ENABLE_BOOT 0x0080
> #define SYSRQ_ENABLE_RTNICE 0x0100
>
> +#define SYSRQ_DISABLE_USERSPACE 0x00010000
> +
> struct sysrq_key_op {
> void (*handler)(int);
> char *help_msg;
> @@ -43,8 +45,12 @@ struct sysrq_key_op {
> * are available -- else NULL's).
> */
>
> +#define SYSRQ_FROM_KERNEL 0x0001
> +#define SYSRQ_FROM_PROC 0x0002
> +#define SYSRQ_FROM_SYNTHETIC 0x0004
> +
> void handle_sysrq(int key);
> -void __handle_sysrq(int key, bool check_mask);
> +void __handle_sysrq(int key, unsigned int from);
> int register_sysrq_key(int key, struct sysrq_key_op *op);
> int unregister_sysrq_key(int key, struct sysrq_key_op *op);
> struct sysrq_key_op *__sysrq_get_key_op(int key);
> diff --git a/kernel/debug/kdb/kdb_main.c b/kernel/debug/kdb/kdb_main.c
> index 82a3b32a7cfc..efee1abf5e8e 100644
> --- a/kernel/debug/kdb/kdb_main.c
> +++ b/kernel/debug/kdb/kdb_main.c
> @@ -1981,7 +1981,7 @@ static int kdb_sr(int argc, const char **argv)
> return KDB_ARGCOUNT;
>
> kdb_trap_printk++;
> - __handle_sysrq(*argv[1], check_mask);
> + __handle_sysrq(*argv[1], check_mask ? SYSRQ_FROM_KERNEL : 0);
> kdb_trap_printk--;
>
> return 0;
> diff --git a/security/Kconfig b/security/Kconfig
> index 47dc3403b5af..8346eb883336 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -244,6 +244,15 @@ config LOCK_DOWN_KERNEL_FORCE
> help
> Enable the kernel lock down functionality automatically at boot.
>
> +config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> + bool "Allow the kernel lockdown to be lifted by SysRq"
> + depends on LOCK_DOWN_KERNEL
> + depends on MAGIC_SYSRQ
> + depends on X86
> + help
> + Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
> + combination on a wired keyboard.
> +
> source "security/selinux/Kconfig"
> source "security/smack/Kconfig"
> source "security/tomoyo/Kconfig"
> diff --git a/security/lock_down.c b/security/lock_down.c
> index 13a8228c1034..cfbc2c39712b 100644
> --- a/security/lock_down.c
> +++ b/security/lock_down.c
> @@ -11,8 +11,14 @@
>
> #include <linux/security.h>
> #include <linux/export.h>
> +#include <linux/sysrq.h>
> +#include <asm/setup.h>
>
> +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> +static __read_mostly bool kernel_locked_down;
> +#else
> static __ro_after_init bool kernel_locked_down;
> +#endif
>
> /*
> * Put the kernel into lock-down mode.
> @@ -57,3 +63,44 @@ bool __kernel_is_locked_down(const char *what, bool first)
> return kernel_locked_down;
> }
> EXPORT_SYMBOL(__kernel_is_locked_down);
> +
> +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> +
> +/*
> + * Take the kernel out of lockdown mode.
> + */
> +static void lift_kernel_lockdown(void)
> +{
> + pr_notice("Lifting lockdown\n");
> + kernel_locked_down = false;
> +}
> +
> +/*
> + * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
> + * echoing the appropriate letter into the sysrq-trigger file).
> + */
> +static void sysrq_handle_lockdown_lift(int key)
> +{
> + if (kernel_locked_down)
> + lift_kernel_lockdown();
> +}
> +
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
> +
> +static int __init lockdown_lift_sysrq(void)
> +{
> + if (kernel_locked_down) {
> + lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
> + register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
> + }
> + return 0;
> +}
> +
> +late_initcall(lockdown_lift_sysrq);
> +
> +#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ */
>


--
~Randy

2019-03-07 00:13:19

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

On Wed, Mar 6, 2019 at 4:10 PM Randy Dunlap <[email protected]> wrote:
>
> On 3/6/19 3:58 PM, Matthew Garrett wrote:
> > From: Kyle McMartin <[email protected]>
> >
> > Make an option to provide a sysrq key that will lift the kernel lockdown,
> > thereby allowing the running kernel image to be accessed and modified.
>
> You still need to document this in Documentation/admin-guide/sysrq.rst,
> like I mentioned last week.

Hm.. On reflection this patch doesn't make much sense without the
automatic lockdown enable functionality, so I'll just drop it from the
patchset instead. Thanks!

2019-03-07 03:57:27

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PULL REQUEST] Kernel lockdown patches for 5.2

On Wed, 2019-03-06 at 15:58 -0800, Matthew Garrett wrote:

> 3) The integration with IMA has been dropped for now. IMA is in the
> process of adding support for architecture-specific policies that will
> interact correctly with the lockdown feature, and a followup patch will
> integrate that so we don't end up with an ordering dependency on the
> merge

The architecture specific policy is an attempt to coordinate between
the different signature verification methods (eg. PE and IMA kexec
kernel image signatures, appended and IMA kernel module signatures).
 The coordination between these signature verification methods is
independent of the "lockdown" feature.

To prevent requiring multiple signature verifications, an IMA policy
rule(s) is defined only if either KEXEC_VERIFY_SIG or MODULE_SIG is
not enabled.

The kexec and kernel modules patches in this patch set continues to
ignore IMA.  This patch set should up front either provide an
alternative solution to coordinate the different signature
verification methods or rely on the architecture specific policy for
that coordination.

Mimi


2019-03-07 04:25:37

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PULL REQUEST] Kernel lockdown patches for 5.2

On Wed, Mar 6, 2019 at 7:56 PM Mimi Zohar <[email protected]> wrote:
> The kexec and kernel modules patches in this patch set continues to
> ignore IMA. This patch set should up front either provide an
> alternative solution to coordinate the different signature
> verification methods or rely on the architecture specific policy for
> that coordination.

Hi Mimi,

I'm working on a patch for this at the moment which can then be added
to either patchset. Is there a tree that contains the proposed Power
architecture policy? I want to make sure I don't accidentally end up
depending on anything x86.

2019-03-07 14:56:14

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down

On Wed, 6 Mar 2019 15:58:55 -0800
Matthew Garrett <[email protected]> wrote:

> From: Josh Boyer <[email protected]>
>
> There is currently no way to verify the resume image when returning
> from hibernate. This might compromise the signed modules trust model,
> so until we can work with signed hibernate images we disable it when the
> kernel is locked down.

That one is a bit worrying since whilst the other stuff may be useful in
some business environments, mandatory hibernate not suspend to RAM is a
common corporate IT policy because of concerns about theft and recovery
of memory contents.

Alan

2019-03-07 16:01:23

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

Matthew Garrett <[email protected]> wrote:

> + /* Ban synthetic events from some sysrq functionality */
> + if ((from == SYSRQ_FROM_PROC || from == SYSRQ_FROM_SYNTHETIC) &&
> + op_p->enable_mask & SYSRQ_DISABLE_USERSPACE)
> + printk("This sysrq operation is disabled from userspace.\n");
> /*
> * Should we check for enabled operations (/proc/sysrq-trigger
> * should not) and is the invoked operation enabled?
> */
> - if (!check_mask || sysrq_on_mask(op_p->enable_mask)) {
> + if (from == SYSRQ_FROM_KERNEL || sysrq_on_mask(op_p->enable_mask)) {

There's some missing logic here. Probably an else is missing, but it seems
more than that.

David

2019-03-07 17:33:32

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down

On Thu, Mar 7, 2019 at 6:55 AM Alan Cox <[email protected]> wrote:
>
> On Wed, 6 Mar 2019 15:58:55 -0800
> Matthew Garrett <[email protected]> wrote:
>
> > From: Josh Boyer <[email protected]>
> >
> > There is currently no way to verify the resume image when returning
> > from hibernate. This might compromise the signed modules trust model,
> > so until we can work with signed hibernate images we disable it when the
> > kernel is locked down.
>
> That one is a bit worrying since whilst the other stuff may be useful in
> some business environments, mandatory hibernate not suspend to RAM is a
> common corporate IT policy because of concerns about theft and recovery
> of memory contents.

Suse have a solution for this that I'd like to see pushed again, but
from a practical perspective enterprise distributions have been
shipping this for some time without significant obvious customer
complaint.

2019-03-08 23:01:25

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Wed, 6 Mar 2019, Matthew Garrett wrote:

> From: David Howells <[email protected]>
>
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.

Perhaps note that this won't cover the case where folk are using DM-Verity
with a signed root hash for verifying kernel modules.


--
James Morris
<[email protected]>


2019-03-08 23:31:38

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Fri, Mar 8, 2019 at 3:00 PM James Morris <[email protected]> wrote:
>
> On Wed, 6 Mar 2019, Matthew Garrett wrote:
>
> > From: David Howells <[email protected]>
> >
> > If the kernel is locked down, require that all modules have valid
> > signatures that we can verify.
>
> Perhaps note that this won't cover the case where folk are using DM-Verity
> with a signed root hash for verifying kernel modules.

Mm. I can't see a terribly good way of doing this generically -
loadpin gives no indication to the module loading code that it comes
from a trusted source. Would making the lockdown/module signature
enforcement a separate config option be reasonable?

2019-03-09 04:46:31

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Fri, 8 Mar 2019, Matthew Garrett wrote:

> On Fri, Mar 8, 2019 at 3:00 PM James Morris <[email protected]> wrote:
> >
> > On Wed, 6 Mar 2019, Matthew Garrett wrote:
> >
> > > From: David Howells <[email protected]>
> > >
> > > If the kernel is locked down, require that all modules have valid
> > > signatures that we can verify.
> >
> > Perhaps note that this won't cover the case where folk are using DM-Verity
> > with a signed root hash for verifying kernel modules.
>
> Mm. I can't see a terribly good way of doing this generically -
> loadpin gives no indication to the module loading code that it comes
> from a trusted source. Would making the lockdown/module signature
> enforcement a separate config option be reasonable?

I was just suggest documenting this.

--
James Morris
<[email protected]>


2019-03-12 00:43:36

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PULL REQUEST] Kernel lockdown patches for 5.2

On Wed, Mar 6, 2019 at 8:24 PM Matthew Garrett <[email protected]> wrote:
>
> On Wed, Mar 6, 2019 at 7:56 PM Mimi Zohar <[email protected]> wrote:
> > The kexec and kernel modules patches in this patch set continues to
> > ignore IMA. This patch set should up front either provide an
> > alternative solution to coordinate the different signature
> > verification methods or rely on the architecture specific policy for
> > that coordination.
>
> Hi Mimi,
>
> I'm working on a patch for this at the moment which can then be added
> to either patchset. Is there a tree that contains the proposed Power
> architecture policy? I want to make sure I don't accidentally end up
> depending on anything x86.

I've been digging into this some more, and want to ensure that I get
the appropriate semantics. Are we happy with the x86 solution for
module signing (ie, if the arch policy is enabled and the kernel
supports module signatures, use module signatures rather than IMA
signatures)? If so, that just leaves kexec. For platforms that support
PE signing for kernels (x86 and arm), are we ok punting to that? If so
then to maintain the semantics we have for lockdown in general (ie, no
way for a user to modify ring 0 code) then I think that would mean
allowing kexec_file() only when the following criteria are met:

1) IMA is appraising kexec with digital signatures, either ima digital
signatures or ima hashes with associated EVM digital signatures
2) CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an
attacker being able to add a key to the keyring

Does this sound reasonable? Are there any further criteria that are
required for this?

2019-03-12 01:54:08

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PULL REQUEST] Kernel lockdown patches for 5.2

On Mon, 2019-03-11 at 17:42 -0700, Matthew Garrett wrote:
> On Wed, Mar 6, 2019 at 8:24 PM Matthew Garrett <[email protected]> wrote:
> >
> > On Wed, Mar 6, 2019 at 7:56 PM Mimi Zohar <[email protected]> wrote:
> > > The kexec and kernel modules patches in this patch set continues to
> > > ignore IMA. This patch set should up front either provide an
> > > alternative solution to coordinate the different signature
> > > verification methods or rely on the architecture specific policy for
> > > that coordination.
> >
> > Hi Mimi,
> >
> > I'm working on a patch for this at the moment which can then be added
> > to either patchset. Is there a tree that contains the proposed Power
> > architecture policy? I want to make sure I don't accidentally end up
> > depending on anything x86.
>
> I've been digging into this some more, and want to ensure that I get
> the appropriate semantics. Are we happy with the x86 solution for
> module signing (ie, if the arch policy is enabled and the kernel
> supports module signatures, use module signatures rather than IMA
> signatures)?

There's a slight nuance you're missing.  If the arch policy is enabled
and the kernel supports module signatures, do not add an IMA appraise
rule.  A custom policy could require an IMA signature, as well as the
module appended signature.

Saying only use the module signatures, even if the IMA custom policy
contains a kernel module rule, doesn't make sense.

> If so, that just leaves kexec. For platforms that support
> PE signing for kernels (x86 and arm), are we ok punting to that?

Similarly, if the custom policy has a kexec kernel image policy rule,
it shouldn't be ignored.

> If so
> then to maintain the semantics we have for lockdown in general (ie, no
> way for a user to modify ring 0 code) then I think that would mean
> allowing kexec_file() only when the following criteria are met:
>
> 1) IMA is appraising kexec with digital signatures, either ima digital
> signatures or ima hashes with associated EVM digital signatures

The kernel image could be signed with an appended signature as well.

> 2) CONFIG_INTEGRITY_TRUSTED_KEYRING is set in order to prevent an
> attacker being able to add a key to the keyring

Agreed

> Does this sound reasonable? Are there any further criteria that are
> required for this?

With the caveats described above.

Mimi


2019-03-18 18:57:19

by Alan Cox

[permalink] [raw]
Subject: Re: [PATCH 09/27] hibernate: Disable when the kernel is locked down

> Suse have a solution for this that I'd like to see pushed again, but
> from a practical perspective enterprise distributions have been
> shipping this for some time without significant obvious customer
> complaint.

Probably because their IT department hasn't noticed 8)

Alan

2019-04-25 13:26:41

by Vasily Gorbik

[permalink] [raw]
Subject: Re: [PATCH 26/27] debugfs: Restrict debugfs when the kernel is locked down

On Wed, Mar 06, 2019 at 03:59:12PM -0800, Matthew Garrett wrote:
> From: David Howells <[email protected]>
>
> static int open_proxy_open(struct inode *inode, struct file *filp)
> {
> struct dentry *dentry = F_DENTRY(filp);
> @@ -147,6 +166,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
> return r == -EIO ? -ENOENT : r;
>
> real_fops = debugfs_real_fops(filp);
> +
> + r = -EPERM;
> + if (debugfs_is_locked_down(inode, filp, real_fops))
> + goto out;
> +
> real_fops = fops_get(real_fops);
> if (!real_fops) {
> /* Huh? Module did not clean up after itself at exit? */
> @@ -272,6 +296,10 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
> return r == -EIO ? -ENOENT : r;
>
> real_fops = debugfs_real_fops(filp);
> + r = -EPERM;
> + if (debugfs_is_locked_down(inode, filp, real_fops))
> + goto out;
> +
> real_fops = fops_get(real_fops);
> if (!real_fops) {
> /* Huh? Module did not cleanup after itself at exit? */

Please be aware that this patch has been known to cause problems in
distributions which picked this patch series already:
ubuntu:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1807686
fedora:
https://bugzilla.redhat.com/show_bug.cgi?id=1658675
as well as rhel 8.

I've sent around this potential fix which has been picked by the
distributions (offered via bugzillas), but went apparently unnoticed
at lkml:
https://lkml.org/lkml/2018/11/21/634
https://lkml.org/lkml/2018/11/21/635

"""
With "debugfs: Restrict debugfs when the kernel is locked down"
return code "r" is unconditionally set to -EPERM, which stays like that
until function return if no "open" file operation defined, effectivelly
resulting in "Operation not permitted" for all such files despite kernel
lock down status or CONFIG_LOCK_DOWN_KERNEL being enabled.
"""

I would appreciate if you consider that change, possibly just
squashing into yours.

2019-04-25 23:01:59

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH 26/27] debugfs: Restrict debugfs when the kernel is locked down

On Thu, Apr 25, 2019 at 3:49 AM Vasily Gorbik <[email protected]> wrote:
> I would appreciate if you consider that change, possibly just
> squashing into yours.

Thanks, I've squashed that in.

2017-11-08 03:13:29

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown


Thiago Jung Bauermann <[email protected]> wrote:

> On non-x86 platforms (tested on powerpc) this fails to build with:
>
> security/lock_down.c: In function ‘lockdown_lift_sysrq’:
> security/lock_down.c:100:40: error: ‘LOCKDOWN_LIFT_KEY’ undeclared (first use in this function)
> lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
> ^~~~~~~~~~~~~~~~~
> security/lock_down.c:100:40: note: each undeclared identifier is reported only once for each function it appears in

I've added an arch dependency in the Kconfig file in my local branch. I'll
try to get it pushed again.

David

From 1583461664472095658@xxx Wed Nov 08 02:05:20 +0000 2017
X-GM-THRID: 1581705988971189585
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-08 02:05:20

by Thiago Jung Bauermann

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown


Hello David,

David Howells <[email protected]> writes:
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
> +
> +static int __init lockdown_lift_sysrq(void)
> +{
> + if (kernel_locked_down) {
> + lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
> + register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
> + }
> + return 0;
> +}
> +
> +late_initcall(lockdown_lift_sysrq);
> +
> +#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_KEY */

On non-x86 platforms (tested on powerpc) this fails to build with:

security/lock_down.c: In function ‘lockdown_lift_sysrq’:
security/lock_down.c:100:40: error: ‘LOCKDOWN_LIFT_KEY’ undeclared (first use in this function)
lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
^~~~~~~~~~~~~~~~~
security/lock_down.c:100:40: note: each undeclared identifier is reported only once for each function it appears in

--
Thiago Jung Bauermann
IBM Linux Technology Center


From 1581728061667553727@xxx Thu Oct 19 22:50:27 +0000 2017
X-GM-THRID: 1581705988971189585
X-Gmail-Labels: Inbox,Category Forums

2017-10-19 22:50:28

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

Randy Dunlap <[email protected]> wrote:

> > +config ALLOW_LOCKDOWN_LIFT
> > + bool
> > + help
> > + Allow the lockdown on a kernel to be lifted, thereby restoring the
> > + ability of userspace to access the kernel image (eg. by SysRq+x under
>
> how about: on
>
> > + x86).

I'll just get rid of this config option, I think - it doesn't make anything
available outside of lock_down.c.

> > +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_KEY
>
> is that the same as: CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ ?
> tested?

My test machine doesn't have a physical keyboard attached, but you're right.

David

From 1581708820622906590@xxx Thu Oct 19 17:44:38 +0000 2017
X-GM-THRID: 1581705988971189585
X-Gmail-Labels: Inbox,Category Forums

2017-10-19 17:44:38

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH 02/27] Add a SysRq option to lift kernel lockdown

On 10/19/17 07:50, David Howells wrote:
> From: Kyle McMartin <[email protected]>
>
> Make an option to provide a sysrq key that will lift the kernel lockdown,
> thereby allowing the running kernel image to be accessed and modified.
>
> On x86_64 this is triggered with SysRq+x, but this key may not be available
> on all arches, so it is set by setting LOCKDOWN_LIFT_KEY in asm/setup.h.
>
> Signed-off-by: Kyle McMartin <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
>
> arch/x86/include/asm/setup.h | 2 ++
> drivers/input/misc/uinput.c | 1 +
> drivers/tty/sysrq.c | 19 +++++++++++------
> include/linux/input.h | 5 ++++
> include/linux/sysrq.h | 8 ++++++-
> kernel/debug/kdb/kdb_main.c | 2 +-
> security/Kconfig | 15 +++++++++++++
> security/lock_down.c | 48 ++++++++++++++++++++++++++++++++++++++++++
> 8 files changed, 92 insertions(+), 8 deletions(-)

> diff --git a/security/Kconfig b/security/Kconfig
> index 8e01fd59ae7e..4be6be71e075 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -213,6 +213,21 @@ config LOCK_DOWN_KERNEL
> turns off various features that might otherwise allow access to the
> kernel image (eg. setting MSR registers).
>
> +config ALLOW_LOCKDOWN_LIFT
> + bool
> + help
> + Allow the lockdown on a kernel to be lifted, thereby restoring the
> + ability of userspace to access the kernel image (eg. by SysRq+x under

how about: on

> + x86).
> +
> +config ALLOW_LOCKDOWN_LIFT_BY_SYSRQ
> + bool "Allow the kernel lockdown to be lifted by SysRq"
> + depends on MAGIC_SYSRQ
> + help
> + Allow the lockdown on a kernel to be lifted, by pressing a SysRq key
> + combination on a wired keyboard.
> +
> +
> source security/selinux/Kconfig
> source security/smack/Kconfig
> source security/tomoyo/Kconfig
> diff --git a/security/lock_down.c b/security/lock_down.c
> index d8595c0e6673..f71118c340d2 100644
> --- a/security/lock_down.c
> +++ b/security/lock_down.c


> +
> +/*
> + * Allow lockdown to be lifted by pressing something like SysRq+x (and not by
> + * echoing the appropriate letter into the sysrq-trigger file).
> + */
> +#ifdef CONFIG_ALLOW_LOCKDOWN_LIFT_BY_KEY

is that the same as: CONFIG_ALLOW_LOCKDOWN_LIFT_BY_SYSRQ ?
tested?

> +
> +static void sysrq_handle_lockdown_lift(int key)
> +{
> + if (kernel_locked_down)
> + lift_kernel_lockdown();
> +}
> +
> +static struct sysrq_key_op lockdown_lift_sysrq_op = {
> + .handler = sysrq_handle_lockdown_lift,
> + .help_msg = "unSB(x)",
> + .action_msg = "Disabling Secure Boot restrictions",
> + .enable_mask = SYSRQ_DISABLE_USERSPACE,
> +};
> +
> +static int __init lockdown_lift_sysrq(void)
> +{
> + if (kernel_locked_down) {
> + lockdown_lift_sysrq_op.help_msg[5] = LOCKDOWN_LIFT_KEY;
> + register_sysrq_key(LOCKDOWN_LIFT_KEY, &lockdown_lift_sysrq_op);
> + }
> + return 0;
> +}
> +
> +late_initcall(lockdown_lift_sysrq);
> +
> +#endif /* CONFIG_ALLOW_LOCKDOWN_LIFT_BY_KEY */

BY_SYSRQ


--
~Randy

From 1581705988971189585@xxx Thu Oct 19 16:59:37 +0000 2017
X-GM-THRID: 1581705988971189585
X-Gmail-Labels: Inbox,Category Forums

2017-11-02 22:32:10

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Thu, 2017-11-02 at 22:01 +0000, David Howells wrote:
> Mimi Zohar <[email protected]> wrote:
>
> > Right, it would never get here if the IMA signature verification
> > fails.  If sig_enforce is not enabled, then it will also work.  So the
> > only case is if sig_enforced is enabled and there is no key.
> >
> > eg.
> >          else if (can_do_ima_check && is_ima_appraise_enabled())
> >                 err = 0;
>
> I'm not sure where you want to put that, but I can't just do this:
>
> /* Not having a signature is only an error if we're strict. */
> if (err == -ENOKEY && !sig_enforce &&
> (!can_do_ima_check || !is_ima_appraise_enabled()) &&

The above IMA checks aren't needed here.

> !kernel_is_locked_down("Loading of unsigned modules"))
> err = 0;
> else if (can_do_ima_check && is_ima_appraise_enabled())
> err = 0;
>
> because that'll print out a message in lockdown mode saying that you're not
> allowed to do that and then maybe do it anyway.

Then at least for now, document that even though kernel modules might
be signed and verified by IMA-appraisal, that in lockdown mode they
also require an appended signature.

Mimi


From 1582993391809714294@xxx Thu Nov 02 22:02:20 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-02 22:02:20

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

Mimi Zohar <[email protected]> wrote:

> Right, it would never get here if the IMA signature verification
> fails.  If sig_enforce is not enabled, then it will also work.  So the
> only case is if sig_enforced is enabled and there is no key.
>
> eg.
>          else if (can_do_ima_check && is_ima_appraise_enabled())
>                 err = 0;

I'm not sure where you want to put that, but I can't just do this:

/* Not having a signature is only an error if we're strict. */
if (err == -ENOKEY && !sig_enforce &&
(!can_do_ima_check || !is_ima_appraise_enabled()) &&
!kernel_is_locked_down("Loading of unsigned modules"))
err = 0;
else if (can_do_ima_check && is_ima_appraise_enabled())
err = 0;

because that'll print out a message in lockdown mode saying that you're not
allowed to do that and then maybe do it anyway.

David

From 1582992220635451773@xxx Thu Nov 02 21:43:43 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-02 21:43:44

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Thu, 2017-11-02 at 21:30 +0000, David Howells wrote:
> Mimi Zohar <[email protected]> wrote:
>
> > By this point, IMA-appraisal has already verified the kernel module
> > signature back in kernel_read_file_from_fd(), if it was required.
> >  Having a key with which to verify the appended signature or requiring
> > an appended signature, should not be required as well.
>
> I guess I don't need to put in any support for IMA here, then, and you've
> taken care of it in your patchset such that it won't actually go into
> module_sig_check() in that case (or will at least return immediately).

Right, it would never get here if the IMA signature verification
fails.  If sig_enforce is not enabled, then it will also work.  So the
only case is if sig_enforced is enabled and there is no key.

eg.
         else if (can_do_ima_check && is_ima_appraise_enabled())
                err = 0;

Mimi 


From 1582991470215118646@xxx Thu Nov 02 21:31:48 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-02 21:31:48

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

Mimi Zohar <[email protected]> wrote:

> By this point, IMA-appraisal has already verified the kernel module
> signature back in kernel_read_file_from_fd(), if it was required.
>  Having a key with which to verify the appended signature or requiring
> an appended signature, should not be required as well.

I guess I don't need to put in any support for IMA here, then, and you've
taken care of it in your patchset such that it won't actually go into
module_sig_check() in that case (or will at least return immediately).

David

From 1582982856282642877@xxx Thu Nov 02 19:14:53 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-02 19:14:53

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Thu, 2017-11-02 at 17:22 +0000, David Howells wrote:

> #ifdef CONFIG_MODULE_SIG
> -static int module_sig_check(struct load_info *info, int flags)
> +static int module_sig_check(struct load_info *info, int flags,
> + bool can_do_ima_check)
> {
> int err = -ENOKEY;
> const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> @@ -2781,13 +2783,16 @@ static int module_sig_check(struct load_info *info, int flags)
> }
>
> /* Not having a signature is only an error if we're strict. */
> - if (err == -ENOKEY && !sig_enforce)
> + if (err == -ENOKEY && !sig_enforce &&
> + (!can_do_ima_check || !is_ima_appraise_enabled()) &&
> + !kernel_is_locked_down("Loading of unsigned modules"))

By this point, IMA-appraisal has already verified the kernel module
signature back in kernel_read_file_from_fd(), if it was required.
 Having a key with which to verify the appended signature or requiring
an appended signature, should not be required as well.

Mimi


From 1582975848619327419@xxx Thu Nov 02 17:23:30 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread

2017-11-02 17:23:30

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

Hi Mimi,

I've altered this patch to allow for IMA appraisal on finit_module(). See the
attached.

David
---
commit c0d5336356004e7543314e388755a00e725521da
Author: David Howells <[email protected]>
Date: Wed May 24 14:56:01 2017 +0100

Enforce module signatures if the kernel is locked down

If the kernel is locked down, require that all modules have valid
signatures that we can verify or that IMA can validate the file.

Signed-off-by: David Howells <[email protected]>
Reviewed-by: "Lee, Chun-Yi" <[email protected]>
Reviewed-by: James Morris <[email protected]>

diff --git a/kernel/module.c b/kernel/module.c
index de66ec825992..0ce29c8aa75a 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -64,6 +64,7 @@
#include <linux/bsearch.h>
#include <linux/dynamic_debug.h>
#include <linux/audit.h>
+#include <linux/ima.h>
#include <uapi/linux/module.h>
#include "module-internal.h"

@@ -2757,7 +2758,8 @@ static inline void kmemleak_load_module(const struct module *mod,
#endif

#ifdef CONFIG_MODULE_SIG
-static int module_sig_check(struct load_info *info, int flags)
+static int module_sig_check(struct load_info *info, int flags,
+ bool can_do_ima_check)
{
int err = -ENOKEY;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
@@ -2781,13 +2783,16 @@ static int module_sig_check(struct load_info *info, int flags)
}

/* Not having a signature is only an error if we're strict. */
- if (err == -ENOKEY && !sig_enforce)
+ if (err == -ENOKEY && !sig_enforce &&
+ (!can_do_ima_check || !is_ima_appraise_enabled()) &&
+ !kernel_is_locked_down("Loading of unsigned modules"))
err = 0;

return err;
}
#else /* !CONFIG_MODULE_SIG */
-static int module_sig_check(struct load_info *info, int flags)
+static int module_sig_check(struct load_info *info, int flags,
+ bool can_do_ima_check)
{
return 0;
}
@@ -3630,13 +3635,13 @@ static int unknown_module_param_cb(char *param, char *val, const char *modname,
/* Allocate and load the module: note that size of section 0 is always
zero, and we rely on this for optional sections. */
static int load_module(struct load_info *info, const char __user *uargs,
- int flags)
+ int flags, bool can_do_ima_check)
{
struct module *mod;
long err;
char *after_dashes;

- err = module_sig_check(info, flags);
+ err = module_sig_check(info, flags, can_do_ima_check);
if (err)
goto free_copy;

@@ -3830,7 +3835,7 @@ SYSCALL_DEFINE3(init_module, void __user *, umod,
if (err)
return err;

- return load_module(&info, uargs, 0);
+ return load_module(&info, uargs, 0, false);
}

SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
@@ -3857,7 +3862,7 @@ SYSCALL_DEFINE3(finit_module, int, fd, const char __user *, uargs, int, flags)
info.hdr = hdr;
info.len = size;

- return load_module(&info, uargs, flags);
+ return load_module(&info, uargs, flags, true);
}

static inline int within(unsigned long addr, void *start, unsigned long size)

From 1582706031634685285@xxx Mon Oct 30 17:54:52 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums

2017-10-30 17:54:52

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

[Corrected Matthew Garrett's email address.  Cc'ed Bruno Meneguele]

On Mon, 2017-10-30 at 17:00 +0000, David Howells wrote:
> Mimi Zohar <[email protected]> wrote:
>
> > This kernel_is_locked_down() check is being called for both the
> > original and new module_load syscalls.  We need to be able
> > differentiate them.  This is fine for the original syscall, but for
> > the new syscall we would need an additional IMA check -
> > !is_ima_appraise_enabled().
>
> IMA can only be used with finit_module()?

Yes, without the file descriptor, IMA-appraisal can't access the
xattrs. 

You should really look at Bruno's patches, which are in my next
branch:

8168913c50d5 "ima: check signature enforcement against cmdline param instead of CONFIG"
404090509894 module: export module signature enforcement status

Can we get an Ack on the module one?

Mimi


From 1582702702544935623@xxx Mon Oct 30 17:01:57 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums

2017-10-30 17:01:57

by David Howells

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

Mimi Zohar <[email protected]> wrote:

> This kernel_is_locked_down() check is being called for both the
> original and new module_load syscalls.  We need to be able
> differentiate them.  This is fine for the original syscall, but for
> the new syscall we would need an additional IMA check -
> !is_ima_appraise_enabled().

IMA can only be used with finit_module()?

David

From 1582437665954608591@xxx Fri Oct 27 18:49:19 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums

2017-10-27 18:49:19

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Thu, 2017-10-19 at 15:50 +0100, David Howells wrote:
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
>
> Signed-off-by: David Howells <[email protected]>
> ---
>
> kernel/module.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec825992..3d9a3270c179 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2781,7 +2781,8 @@ static int module_sig_check(struct load_info *info, int flags)
> }
>
> /* Not having a signature is only an error if we're strict. */
> - if (err == -ENOKEY && !sig_enforce)
> + if (err == -ENOKEY && !sig_enforce &&
> + !kernel_is_locked_down("Loading of unsigned modules"))
 
This kernel_is_locked_down() check is being called for both the
original and new module_load syscalls.  We need to be able
differentiate them.  This is fine for the original syscall, but for
the new syscall we would need an additional IMA check -
!is_ima_appraise_enabled().

Mimi
 
> err = 0;
>
> return err;


From 1581820698923712657@xxx Fri Oct 20 23:22:53 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums

2017-10-20 23:22:53

by James Morris

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

On Thu, 19 Oct 2017, David Howells wrote:

> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
>
> Signed-off-by: David Howells <[email protected]>


Reviewed-by: James Morris <[email protected]>

--
James Morris
<[email protected]>


From 1581757281914087642@xxx Fri Oct 20 06:34:54 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums

2017-10-20 06:34:55

by joeyli

[permalink] [raw]
Subject: Re: [PATCH 03/27] Enforce module signatures if the kernel is locked down

Hi David,

Thanks for you send our this series.

On Thu, Oct 19, 2017 at 03:50:55PM +0100, David Howells wrote:
> If the kernel is locked down, require that all modules have valid
> signatures that we can verify.
>
> Signed-off-by: David Howells <[email protected]>

I have reviewed and tested this patch. Please feel free to add:

Reviewed-by: "Lee, Chun-Yi" <[email protected]>

Thanks a lot!
Joey Lee

> ---
>
> kernel/module.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index de66ec825992..3d9a3270c179 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2781,7 +2781,8 @@ static int module_sig_check(struct load_info *info, int flags)
> }
>
> /* Not having a signature is only an error if we're strict. */
> - if (err == -ENOKEY && !sig_enforce)
> + if (err == -ENOKEY && !sig_enforce &&
> + !kernel_is_locked_down("Loading of unsigned modules"))
> err = 0;
>
> return err;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

From 1581705993536835286@xxx Thu Oct 19 16:59:42 +0000 2017
X-GM-THRID: 1581705993536835286
X-Gmail-Labels: Inbox,Category Forums