Minor updates over V33 - security_is_locked_down renamed to
security_locked_down, return value of security_locked_down is returned
in most cases, one unnecessary patch was dropped, couple of minor nits
fixed.
The lockdown module is intended to allow for kernels to be locked down
early in boot - sufficiently early that we don't have the ability to
kmalloc() yet. Add support for early initialisation of some LSMs, and
then add them to the list of names when we do full initialisation later.
Early LSMs are initialised in link order and cannot be overridden via
boot parameters, and cannot make use of kmalloc() (since the allocator
isn't initialised yet).
Signed-off-by: Matthew Garrett <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 8 ++++-
include/linux/lsm_hooks.h | 6 ++++
include/linux/security.h | 1 +
init/main.c | 1 +
security/security.c | 50 ++++++++++++++++++++++++++-----
5 files changed, 57 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index f8f6f04c4453..e1963352fdb6 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -208,8 +208,13 @@
__start_lsm_info = .; \
KEEP(*(.lsm_info.init)) \
__end_lsm_info = .;
+#define EARLY_LSM_TABLE() . = ALIGN(8); \
+ __start_early_lsm_info = .; \
+ KEEP(*(.early_lsm_info.init)) \
+ __end_early_lsm_info = .;
#else
#define LSM_TABLE()
+#define EARLY_LSM_TABLE()
#endif
#define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
@@ -610,7 +615,8 @@
ACPI_PROBE_TABLE(irqchip) \
ACPI_PROBE_TABLE(timer) \
EARLYCON_TABLE() \
- LSM_TABLE()
+ LSM_TABLE() \
+ EARLY_LSM_TABLE()
#define INIT_TEXT \
*(.init.text .init.text.*) \
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index a240a3fc5fc4..66fd1eac7a32 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -2085,12 +2085,18 @@ struct lsm_info {
};
extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
+extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
#define DEFINE_LSM(lsm) \
static struct lsm_info __lsm_##lsm \
__used __section(.lsm_info.init) \
__aligned(sizeof(unsigned long))
+#define DEFINE_EARLY_LSM(lsm) \
+ static struct lsm_info __early_lsm_##lsm \
+ __used __section(.early_lsm_info.init) \
+ __aligned(sizeof(unsigned long))
+
#ifdef CONFIG_SECURITY_SELINUX_DISABLE
/*
* Assuring the safety of deleting a security module is up to
diff --git a/include/linux/security.h b/include/linux/security.h
index 49f2685324b0..1bb6fb2f1523 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -194,6 +194,7 @@ int unregister_lsm_notifier(struct notifier_block *nb);
/* prototypes */
extern int security_init(void);
+extern int early_security_init(void);
/* Security operations */
int security_binder_set_context_mgr(struct task_struct *mgr);
diff --git a/init/main.c b/init/main.c
index 598e278b46f7..f3faeb89c75f 100644
--- a/init/main.c
+++ b/init/main.c
@@ -563,6 +563,7 @@ asmlinkage __visible void __init start_kernel(void)
boot_cpu_init();
page_address_init();
pr_notice("%s", linux_banner);
+ early_security_init();
setup_arch(&command_line);
/*
* Set up the the initial canary and entropy after arch
diff --git a/security/security.c b/security/security.c
index 23cbb1a295a3..487e1f3eb2df 100644
--- a/security/security.c
+++ b/security/security.c
@@ -37,6 +37,7 @@
/* How many LSMs were built into the kernel? */
#define LSM_COUNT (__end_lsm_info - __start_lsm_info)
+#define EARLY_LSM_COUNT (__end_early_lsm_info - __start_early_lsm_info)
struct security_hook_heads security_hook_heads __lsm_ro_after_init;
static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
@@ -281,6 +282,8 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
static void __init lsm_early_cred(struct cred *cred);
static void __init lsm_early_task(struct task_struct *task);
+static int lsm_append(const char *new, char **result);
+
static void __init ordered_lsm_init(void)
{
struct lsm_info **lsm;
@@ -327,6 +330,26 @@ static void __init ordered_lsm_init(void)
kfree(ordered_lsms);
}
+int __init early_security_init(void)
+{
+ int i;
+ struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
+ struct lsm_info *lsm;
+
+ for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
+ i++)
+ INIT_HLIST_HEAD(&list[i]);
+
+ for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
+ if (!lsm->enabled)
+ lsm->enabled = &lsm_enabled_true;
+ prepare_lsm(lsm);
+ initialize_lsm(lsm);
+ }
+
+ return 0;
+}
+
/**
* security_init - initializes the security framework
*
@@ -334,14 +357,18 @@ static void __init ordered_lsm_init(void)
*/
int __init security_init(void)
{
- int i;
- struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
+ struct lsm_info *lsm;
pr_info("Security Framework initializing\n");
- for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
- i++)
- INIT_HLIST_HEAD(&list[i]);
+ /*
+ * Append the names of the early LSM modules now that kmalloc() is
+ * available
+ */
+ for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
+ if (lsm->enabled)
+ lsm_append(lsm->name, &lsm_names);
+ }
/* Load LSMs in specified order. */
ordered_lsm_init();
@@ -388,7 +415,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
return !strcmp(last, lsm);
}
-static int lsm_append(char *new, char **result)
+static int lsm_append(const char *new, char **result)
{
char *cp;
@@ -426,8 +453,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
hooks[i].lsm = lsm;
hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
}
- if (lsm_append(lsm, &lsm_names) < 0)
- panic("%s - Cannot get early memory.\n", __func__);
+
+ /*
+ * Don't try to append during early_security_init(), we'll come back
+ * and fix this up afterwards.
+ */
+ if (slab_is_available()) {
+ if (lsm_append(lsm, &lsm_names) < 0)
+ panic("%s - Cannot get early memory.\n", __func__);
+ }
}
int call_lsm_notifier(enum lsm_event event, void *data)
--
2.22.0.410.gd8fdbe21b5-goog
Add a mechanism to allow LSMs to make a policy decision around whether
kernel functionality that would allow tampering with or examining the
runtime state of the kernel should be permitted.
Signed-off-by: Matthew Garrett <[email protected]>
---
include/linux/lsm_hooks.h | 2 ++
include/linux/security.h | 11 +++++++++++
security/security.c | 6 ++++++
3 files changed, 19 insertions(+)
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 66fd1eac7a32..df2aebc99838 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -1790,6 +1790,7 @@ union security_list_options {
int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
#endif /* CONFIG_BPF_SYSCALL */
+ int (*locked_down)(enum lockdown_reason what);
};
struct security_hook_heads {
@@ -2027,6 +2028,7 @@ struct security_hook_heads {
struct hlist_head bpf_prog_alloc_security;
struct hlist_head bpf_prog_free_security;
#endif /* CONFIG_BPF_SYSCALL */
+ struct hlist_head locked_down;
} __randomize_layout;
/*
diff --git a/include/linux/security.h b/include/linux/security.h
index 1bb6fb2f1523..9eaf02e70707 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -76,6 +76,12 @@ enum lsm_event {
LSM_POLICY_CHANGE,
};
+enum lockdown_reason {
+ LOCKDOWN_NONE,
+ LOCKDOWN_INTEGRITY_MAX,
+ LOCKDOWN_CONFIDENTIALITY_MAX,
+};
+
/* These functions are in security/commoncap.c */
extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
int cap, unsigned int opts);
@@ -389,6 +395,7 @@ void security_inode_invalidate_secctx(struct inode *inode);
int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
+int security_locked_down(enum lockdown_reason what);
#else /* CONFIG_SECURITY */
static inline int call_lsm_notifier(enum lsm_event event, void *data)
@@ -1189,6 +1196,10 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
{
return -EOPNOTSUPP;
}
+static inline int security_locked_down(enum lockdown_reason what)
+{
+ return 0;
+}
#endif /* CONFIG_SECURITY */
#ifdef CONFIG_SECURITY_NETWORK
diff --git a/security/security.c b/security/security.c
index 487e1f3eb2df..553f50e9a106 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2382,3 +2382,9 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
call_void_hook(bpf_prog_free_security, aux);
}
#endif /* CONFIG_BPF_SYSCALL */
+
+int security_locked_down(enum lockdown_reason what)
+{
+ return call_int_hook(locked_down, 0, what);
+}
+EXPORT_SYMBOL(security_locked_down);
--
2.22.0.410.gd8fdbe21b5-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: [email protected]
---
drivers/char/mem.c | 6 +++++-
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index b08dc50f9f26..93c02493f0fa 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -29,8 +29,8 @@
#include <linux/export.h>
#include <linux/io.h>
#include <linux/uio.h>
-
#include <linux/uaccess.h>
+#include <linux/security.h>
#ifdef CONFIG_IA64
# include <linux/efi.h>
@@ -786,6 +786,10 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
static int open_port(struct inode *inode, struct file *filp)
{
+ int ret = security_locked_down(LOCKDOWN_DEV_MEM);
+
+ if (ret)
+ return ret;
return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
}
diff --git a/include/linux/security.h b/include/linux/security.h
index 46d85cd63b06..200175c8605a 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -83,6 +83,7 @@ enum lsm_event {
enum lockdown_reason {
LOCKDOWN_NONE,
LOCKDOWN_MODULE_SIGNATURE,
+ LOCKDOWN_DEV_MEM,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 25a3a5b0aa9c..565c87451f0f 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -19,6 +19,7 @@ static enum lockdown_reason kernel_locked_down;
static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
+ [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Dave Young <[email protected]>
cc: [email protected]
---
include/linux/security.h | 1 +
kernel/kexec.c | 8 ++++++++
security/lockdown/lockdown.c | 1 +
3 files changed, 10 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 200175c8605a..00a31ab2e5ba 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -84,6 +84,7 @@ enum lockdown_reason {
LOCKDOWN_NONE,
LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_DEV_MEM,
+ LOCKDOWN_KEXEC,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/kexec.c b/kernel/kexec.c
index 68559808fdfa..ec3f07a4b1c0 100644
--- a/kernel/kexec.c
+++ b/kernel/kexec.c
@@ -207,6 +207,14 @@ 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
+ */
+ result = security_locked_down(LOCKDOWN_KEXEC);
+ if (result)
+ return result;
+
/*
* Verify we have a legal set of flags
* This leaves us room for future extensions.
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 565c87451f0f..08fcd8116db3 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -20,6 +20,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
+ [LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [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 22f60dd26460..4243359ac509 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -182,6 +182,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.22.0.410.gd8fdbe21b5-goog
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 if there is a signature for which we
have a key, but signature doesn't match - even if in non-forcing mode.
(3) kexec fails with EBADMSG or some other error if there is a signature
which cannot be parsed - even if in non-forcing mode.
(4) 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]>
Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: [email protected]
---
arch/x86/Kconfig | 20 ++++++++---
crypto/asymmetric_keys/verify_pefile.c | 4 ++-
include/linux/kexec.h | 4 +--
kernel/kexec_file.c | 47 ++++++++++++++++++++++----
4 files changed, 60 insertions(+), 15 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index c1f9b3cf437c..84381dd60760 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -2012,20 +2012,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..eec7e5bb2a08 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,47 @@ 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);
+ 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.22.0.410.gd8fdbe21b5-goog
While existing LSMs can be extended to handle lockdown policy,
distributions generally want to be able to apply a straightforward
static policy. This patch adds a simple LSM that can be configured to
reject either integrity or all lockdown queries, and can be configured
at runtime (through securityfs), boot time (via a kernel parameter) or
build time (via a kconfig option). Based on initial code by David
Howells.
Signed-off-by: Matthew Garrett <[email protected]>
Cc: David Howells <[email protected]>
---
.../admin-guide/kernel-parameters.txt | 9 +
include/linux/security.h | 4 +
security/Kconfig | 3 +-
security/Makefile | 2 +
security/lockdown/Kconfig | 47 +++++
security/lockdown/Makefile | 1 +
security/lockdown/lockdown.c | 172 ++++++++++++++++++
7 files changed, 237 insertions(+), 1 deletion(-)
create mode 100644 security/lockdown/Kconfig
create mode 100644 security/lockdown/Makefile
create mode 100644 security/lockdown/lockdown.c
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb644..fa336f6cd5bc 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2239,6 +2239,15 @@
lockd.nlm_udpport=M [NFS] Assign UDP port.
Format: <integer>
+ lockdown= [SECURITY]
+ { integrity | confidentiality }
+ Enable the kernel lockdown feature. If set to
+ integrity, kernel features that allow userland to
+ modify the running kernel are disabled. If set to
+ confidentiality, kernel features that allow userland
+ to extract confidential information from the kernel
+ are also disabled.
+
locktorture.nreaders_stress= [KNL]
Set the number of locking read-acquisition kthreads.
Defaults to being automatically set based on the
diff --git a/include/linux/security.h b/include/linux/security.h
index 9eaf02e70707..c808d344ec75 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -76,6 +76,10 @@ enum lsm_event {
LSM_POLICY_CHANGE,
};
+/*
+ * If you add to this, remember to extend lockdown_reasons in
+ * security/lockdown/lockdown.c.
+ */
enum lockdown_reason {
LOCKDOWN_NONE,
LOCKDOWN_INTEGRITY_MAX,
diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..c35aa72103df 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -236,12 +236,13 @@ source "security/apparmor/Kconfig"
source "security/loadpin/Kconfig"
source "security/yama/Kconfig"
source "security/safesetid/Kconfig"
+source "security/lockdown/Kconfig"
source "security/integrity/Kconfig"
config LSM
string "Ordered list of enabled LSMs"
- default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+ default "lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
diff --git a/security/Makefile b/security/Makefile
index c598b904938f..be1dd9d2cb2f 100644
--- a/security/Makefile
+++ b/security/Makefile
@@ -11,6 +11,7 @@ subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
subdir-$(CONFIG_SECURITY_YAMA) += yama
subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin
subdir-$(CONFIG_SECURITY_SAFESETID) += safesetid
+subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
# always enable default capabilities
obj-y += commoncap.o
@@ -27,6 +28,7 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
obj-$(CONFIG_SECURITY_YAMA) += yama/
obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/
+obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/
obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
# Object integrity file lists
diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
new file mode 100644
index 000000000000..7374ba76d8eb
--- /dev/null
+++ b/security/lockdown/Kconfig
@@ -0,0 +1,47 @@
+config SECURITY_LOCKDOWN_LSM
+ bool "Basic module for enforcing kernel lockdown"
+ depends on SECURITY
+ help
+ Build support for an LSM that enforces a coarse kernel lockdown
+ behaviour.
+
+config SECURITY_LOCKDOWN_LSM_EARLY
+ bool "Enable lockdown LSM early in init"
+ depends on SECURITY_LOCKDOWN_LSM
+ help
+ Enable the lockdown LSM early in boot. This is necessary in order
+ to ensure that lockdown enforcement can be carried out on kernel
+ boot parameters that are otherwise parsed before the security
+ subsystem is fully initialised. If enabled, lockdown will
+ unconditionally be called before any other LSMs.
+
+choice
+ prompt "Kernel default lockdown mode"
+ default LOCK_DOWN_KERNEL_FORCE_NONE
+ depends on SECURITY_LOCKDOWN_LSM
+ help
+ The kernel can be configured to default to differing levels of
+ lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_NONE
+ bool "None"
+ help
+ No lockdown functionality is enabled by default. Lockdown may be
+ enabled via the kernel commandline or /sys/kernel/security/lockdown.
+
+config LOCK_DOWN_KERNEL_FORCE_INTEGRITY
+ bool "Integrity"
+ help
+ The kernel runs in integrity mode by default. Features that allow
+ the kernel to be modified at runtime are disabled.
+
+config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
+ bool "Confidentiality"
+ help
+ The kernel runs in confidentiality mode by default. Features that
+ allow the kernel to be modified at runtime or that permit userland
+ code to read confidential material held inside the kernel are
+ disabled.
+
+endchoice
+
diff --git a/security/lockdown/Makefile b/security/lockdown/Makefile
new file mode 100644
index 000000000000..e3634b9017e7
--- /dev/null
+++ b/security/lockdown/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown.o
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
new file mode 100644
index 000000000000..8e39b36b8f33
--- /dev/null
+++ b/security/lockdown/lockdown.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+/* 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>
+#include <linux/lsm_hooks.h>
+
+static enum lockdown_reason kernel_locked_down;
+
+static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
+ [LOCKDOWN_NONE] = "none",
+ [LOCKDOWN_INTEGRITY_MAX] = "integrity",
+ [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
+};
+
+static enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
+ LOCKDOWN_INTEGRITY_MAX,
+ LOCKDOWN_CONFIDENTIALITY_MAX};
+
+/*
+ * Put the kernel into lock-down mode.
+ */
+static int lock_kernel_down(const char *where, enum lockdown_reason level)
+{
+ if (kernel_locked_down >= level)
+ return -EPERM;
+
+ kernel_locked_down = level;
+ pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
+ where);
+ return 0;
+}
+
+static int __init lockdown_param(char *level)
+{
+ if (!level)
+ return -EINVAL;
+
+ if (strcmp(level, "integrity") == 0)
+ lock_kernel_down("command line", LOCKDOWN_INTEGRITY_MAX);
+ else if (strcmp(level, "confidentiality") == 0)
+ lock_kernel_down("command line", LOCKDOWN_CONFIDENTIALITY_MAX);
+ else
+ return -EINVAL;
+
+ return 0;
+}
+
+early_param("lockdown", lockdown_param);
+
+/**
+ * lockdown_is_locked_down - Find out if the kernel is locked down
+ * @what: Tag to use in notice generated if lockdown is in effect
+ */
+static int lockdown_is_locked_down(enum lockdown_reason what)
+{
+ if ((kernel_locked_down >= what)) {
+ if (lockdown_reasons[what])
+ pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
+ lockdown_reasons[what]);
+ return -EPERM;
+ }
+
+ return 0;
+}
+
+static struct security_hook_list lockdown_hooks[] __lsm_ro_after_init = {
+ LSM_HOOK_INIT(locked_down, lockdown_is_locked_down),
+};
+
+static int __init lockdown_lsm_init(void)
+{
+#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_INTEGRITY_MAX);
+#elif defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY)
+ lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY_MAX);
+#endif
+ security_add_hooks(lockdown_hooks, ARRAY_SIZE(lockdown_hooks),
+ "lockdown");
+ return 0;
+}
+
+static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
+ loff_t *ppos)
+{
+ char temp[80];
+ int i, offset=0;
+
+ for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
+ enum lockdown_reason level = lockdown_levels[i];
+
+ if (lockdown_reasons[level]) {
+ const char *label = lockdown_reasons[level];
+
+ if (kernel_locked_down == level)
+ offset += sprintf(temp+offset, "[%s] ", label);
+ else
+ offset += sprintf(temp+offset, "%s ", label);
+ }
+ }
+
+ /* Convert the last space to a newline if needed. */
+ if (offset > 0)
+ temp[offset-1] = '\n';
+
+ return simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+}
+
+static ssize_t lockdown_write(struct file *file, const char __user *buf,
+ size_t n, loff_t *ppos)
+{
+ char *state;
+ int i, len, err = -EINVAL;
+
+ state = memdup_user_nul(buf, n);
+ if (IS_ERR(state))
+ return PTR_ERR(state);
+
+ len = strlen(state);
+ if (len && state[len-1] == '\n') {
+ state[len-1] = '\0';
+ len--;
+ }
+
+ for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
+ enum lockdown_reason level = lockdown_levels[i];
+ const char *label = lockdown_reasons[level];
+
+ if (label && !strcmp(state, label))
+ err = lock_kernel_down("securityfs", level);
+ }
+
+ kfree(state);
+ return err ? err : n;
+}
+
+static const struct file_operations lockdown_ops = {
+ .read = lockdown_read,
+ .write = lockdown_write,
+};
+
+static int __init lockdown_secfs_init(void)
+{
+ struct dentry *dentry;
+
+ dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
+ &lockdown_ops);
+ if (IS_ERR(dentry))
+ return PTR_ERR(dentry);
+
+ return 0;
+}
+
+core_initcall(lockdown_secfs_init);
+
+#ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
+DEFINE_EARLY_LSM(lockdown) = {
+#else
+DEFINE_LSM(lockdown) = {
+#endif
+ .name = "lockdown",
+ .init = lockdown_lsm_init,
+};
--
2.22.0.410.gd8fdbe21b5-goog
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: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
cc: [email protected]
---
drivers/pci/pci-sysfs.c | 16 ++++++++++++++++
drivers/pci/proc.c | 14 ++++++++++++--
drivers/pci/syscall.c | 4 +++-
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
5 files changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 25794c27c7a4..e1011efb5a31 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -903,6 +903,11 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
unsigned int size = count;
loff_t init_off = off;
u8 *data = (u8 *) buf;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (off > dev->cfg_size)
return 0;
@@ -1165,6 +1170,11 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
int bar = (unsigned long)attr->private;
enum pci_mmap_state mmap_type;
struct resource *res = &pdev->resource[bar];
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
@@ -1241,6 +1251,12 @@ 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)
{
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
+
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..a72258d70407 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -13,6 +13,7 @@
#include <linux/seq_file.h>
#include <linux/capability.h>
#include <linux/uaccess.h>
+#include <linux/security.h>
#include <asm/byteorder.h>
#include "pci.h"
@@ -115,7 +116,11 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
struct pci_dev *dev = PDE_DATA(ino);
int pos = *ppos;
int size = dev->cfg_size;
- int cnt;
+ int cnt, ret;
+
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
if (pos >= size)
return 0;
@@ -196,6 +201,10 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
#endif /* HAVE_PCI_MMAP */
int ret = 0;
+ ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
+ if (ret)
+ return ret;
+
switch (cmd) {
case PCIIOC_CONTROLLER:
ret = pci_domain_nr(dev->bus);
@@ -237,7 +246,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) ||
+ security_locked_down(LOCKDOWN_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..31e39558d49d 100644
--- a/drivers/pci/syscall.c
+++ b/drivers/pci/syscall.c
@@ -7,6 +7,7 @@
#include <linux/errno.h>
#include <linux/pci.h>
+#include <linux/security.h>
#include <linux/syscalls.h>
#include <linux/uaccess.h>
#include "pci.h"
@@ -90,7 +91,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) ||
+ security_locked_down(LOCKDOWN_PCI_ACCESS))
return -EPERM;
dev = pci_get_domain_bus_and_slot(0, bus, dfn);
diff --git a/include/linux/security.h b/include/linux/security.h
index a051f21a1144..1b849f10dec6 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -86,6 +86,7 @@ enum lockdown_reason {
LOCKDOWN_DEV_MEM,
LOCKDOWN_KEXEC,
LOCKDOWN_HIBERNATION,
+ LOCKDOWN_PCI_ACCESS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index ce5b3da9bd09..e2ee8a16b94c 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -22,6 +22,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_HIBERNATION] = "hibernation",
+ [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: [email protected]
Cc: [email protected]
cc: [email protected]
---
include/linux/security.h | 1 +
kernel/power/hibernate.c | 3 ++-
security/lockdown/lockdown.c | 1 +
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 00a31ab2e5ba..a051f21a1144 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -85,6 +85,7 @@ enum lockdown_reason {
LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_DEV_MEM,
LOCKDOWN_KEXEC,
+ LOCKDOWN_HIBERNATION,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
index abef759de7c8..3a9cb2d3da4a 100644
--- a/kernel/power/hibernate.c
+++ b/kernel/power/hibernate.c
@@ -32,6 +32,7 @@
#include <linux/ctype.h>
#include <linux/genhd.h>
#include <linux/ktime.h>
+#include <linux/security.h>
#include <trace/events/power.h>
#include "power.h"
@@ -70,7 +71,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
bool hibernation_available(void)
{
- return (nohibernate == 0);
+ return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
}
/**
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 08fcd8116db3..ce5b3da9bd09 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
+ [LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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.
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]>
cc: [email protected]
---
arch/x86/kernel/msr.c | 8 ++++++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 10 insertions(+)
diff --git a/arch/x86/kernel/msr.c b/arch/x86/kernel/msr.c
index 4588414e2561..131861b4e447 100644
--- a/arch/x86/kernel/msr.c
+++ b/arch/x86/kernel/msr.c
@@ -39,6 +39,7 @@
#include <linux/notifier.h>
#include <linux/uaccess.h>
#include <linux/gfp.h>
+#include <linux/security.h>
#include <asm/cpufeature.h>
#include <asm/msr.h>
@@ -84,6 +85,10 @@ static ssize_t msr_write(struct file *file, const char __user *buf,
int err = 0;
ssize_t bytes = 0;
+ err = security_locked_down(LOCKDOWN_MSR);
+ if (err)
+ return err;
+
if (count % 8)
return -EINVAL; /* Invalid chunk size */
@@ -135,6 +140,9 @@ static long msr_ioctl(struct file *file, unsigned int ioc, unsigned long arg)
err = -EFAULT;
break;
}
+ err = security_locked_down(LOCKDOWN_MSR);
+ if (err)
+ break;
err = wrmsr_safe_regs_on_cpu(cpu, regs);
if (err)
break;
diff --git a/include/linux/security.h b/include/linux/security.h
index 60569b7e9465..30bc6f058926 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -88,6 +88,7 @@ enum lockdown_reason {
LOCKDOWN_HIBERNATION,
LOCKDOWN_PCI_ACCESS,
LOCKDOWN_IOPORT,
+ LOCKDOWN_MSR,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 895ef3ba1b4c..297a065e6261 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -24,6 +24,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_IOPORT] = "raw io port access",
+ [LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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. This is
a runtime check rather than buildtime in order to allow configurations
where the same kernel may be run in both locked down or permissive modes
depending on local policy.
Suggested-by: Thomas Gleixner <[email protected]>
Signed-off-by: David Howells <[email protected]
Signed-off-by: Matthew Garrett <[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]
---
arch/x86/mm/testmmiotrace.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
index f6ae6830b341..6b9486baa2e9 100644
--- a/arch/x86/mm/testmmiotrace.c
+++ b/arch/x86/mm/testmmiotrace.c
@@ -7,6 +7,7 @@
#include <linux/module.h>
#include <linux/io.h>
#include <linux/mmiotrace.h>
+#include <linux/security.h>
static unsigned long mmio_address;
module_param_hw(mmio_address, ulong, iomem, 0);
@@ -114,6 +115,10 @@ static void do_test_bulk_ioremapping(void)
static int __init init(void)
{
unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
+ int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
+
+ if (ret)
+ return ret;
if (mmio_address == 0) {
pr_err("you have to use the module argument mmio_address.\n");
diff --git a/include/linux/security.h b/include/linux/security.h
index 88064d7f6827..c649cb91e762 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -93,6 +93,7 @@ enum lockdown_reason {
LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_TIOCSSERIAL,
LOCKDOWN_MODULE_PARAMETERS,
+ LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index d03c4c296af7..cd86ed9f4d4b 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -29,6 +29,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
+ [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
From: David Howells <[email protected]>
Disallow access to /proc/kcore when the kernel is locked down to prevent
access to cryptographic data. This is limited to lockdown
confidentiality mode and is still permitted in integrity mode.
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
---
fs/proc/kcore.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
index d29d869abec1..4e95edb1e282 100644
--- a/fs/proc/kcore.c
+++ b/fs/proc/kcore.c
@@ -31,6 +31,7 @@
#include <linux/ioport.h>
#include <linux/memory.h>
#include <linux/sched/task.h>
+#include <linux/security.h>
#include <asm/sections.h>
#include "internal.h"
@@ -545,6 +546,10 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
static int open_kcore(struct inode *inode, struct file *filp)
{
+ int ret = security_locked_down(LOCKDOWN_KCORE);
+
+ if (ret)
+ return ret;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
diff --git a/include/linux/security.h b/include/linux/security.h
index c649cb91e762..3875f6df2ecc 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -95,6 +95,7 @@ enum lockdown_reason {
LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
+ LOCKDOWN_KCORE,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index cd86ed9f4d4b..4c9b324dfc55 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -31,6 +31,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
+ [LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Cc: Peter Zijlstra <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
---
include/linux/security.h | 1 +
kernel/events/core.c | 7 +++++++
security/lockdown/lockdown.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index de0d37b1fe79..53ea85889a48 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -98,6 +98,7 @@ enum lockdown_reason {
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
LOCKDOWN_BPF_READ,
+ LOCKDOWN_PERF,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 72d06e302e99..77f36551756e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -10731,6 +10731,13 @@ SYSCALL_DEFINE5(perf_event_open,
return -EINVAL;
}
+ err = security_locked_down(LOCKDOWN_PERF);
+ if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR))
+ /* REGS_INTR can leak data, lockdown must prevent this */
+ return err;
+ else
+ err = 0;
+
/* Only privileged users can get physical addresses */
if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 2eea2cc13117..a7e75c614416 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -34,6 +34,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
+ [LOCKDOWN_PERF] = "unsafe use of perf",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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/lockdown/lockdown.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 98f9ee0026d5..9ca6f442fbc7 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -83,8 +83,8 @@ static int lockdown_is_locked_down(enum lockdown_reason what)
{
if ((kernel_locked_down >= what)) {
if (lockdown_reasons[what])
- pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
- lockdown_reasons[what]);
+ pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
+ current->comm, lockdown_reasons[what]);
return -EPERM;
}
--
2.22.0.410.gd8fdbe21b5-goog
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]>
cc: [email protected]
---
arch/x86/kernel/ioport.c | 7 +++++--
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
index 0fe1c8782208..61a89d3c0382 100644
--- a/arch/x86/kernel/ioport.c
+++ b/arch/x86/kernel/ioport.c
@@ -11,6 +11,7 @@
#include <linux/errno.h>
#include <linux/types.h>
#include <linux/ioport.h>
+#include <linux/security.h>
#include <linux/smp.h>
#include <linux/stddef.h>
#include <linux/slab.h>
@@ -31,7 +32,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) ||
+ security_locked_down(LOCKDOWN_IOPORT)))
return -EPERM;
/*
@@ -126,7 +128,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) ||
+ security_locked_down(LOCKDOWN_IOPORT))
return -EPERM;
}
regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
diff --git a/include/linux/security.h b/include/linux/security.h
index 1b849f10dec6..60569b7e9465 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -87,6 +87,7 @@ enum lockdown_reason {
LOCKDOWN_KEXEC,
LOCKDOWN_HIBERNATION,
LOCKDOWN_PCI_ACCESS,
+ LOCKDOWN_IOPORT,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index e2ee8a16b94c..895ef3ba1b4c 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -23,6 +23,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_KEXEC] = "kexec of unsigned images",
[LOCKDOWN_HIBERNATION] = "hibernation",
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
+ [LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: Dave Young <[email protected]>
cc: [email protected]
---
drivers/acpi/osl.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index f29e427d0d1d..60cda8a0f36b 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -40,6 +40,7 @@
#include <linux/list.h>
#include <linux/jiffies.h>
#include <linux/semaphore.h>
+#include <linux/security.h>
#include <asm/io.h>
#include <linux/uaccess.h>
@@ -194,7 +195,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
acpi_physical_address pa;
#ifdef CONFIG_KEXEC
- if (acpi_rsdp)
+ if (acpi_rsdp && !security_locked_down(LOCKDOWN_ACPI_TABLES))
return acpi_rsdp;
#endif
pa = acpi_arch_get_root_pointer();
--
2.22.0.410.gd8fdbe21b5-goog
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 | 30 ++++++++++++++++++++++++++++++
fs/debugfs/inode.c | 32 ++++++++++++++++++++++++++++++--
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
4 files changed, 62 insertions(+), 2 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 4fce1da7db23..f60518f0e3aa 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -19,6 +19,7 @@
#include <linux/atomic.h>
#include <linux/device.h>
#include <linux/poll.h>
+#include <linux/security.h>
#include "internal.h"
@@ -136,6 +137,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 security_locked_down(LOCKDOWN_DEBUGFS);
+}
+
static int open_proxy_open(struct inode *inode, struct file *filp)
{
struct dentry *dentry = F_DENTRY(filp);
@@ -147,6 +167,11 @@ static int open_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+
+ r = debugfs_is_locked_down(inode, filp, real_fops);
+ if (r)
+ goto out;
+
real_fops = fops_get(real_fops);
if (!real_fops) {
/* Huh? Module did not clean up after itself at exit? */
@@ -272,6 +297,11 @@ static int full_proxy_open(struct inode *inode, struct file *filp)
return r == -EIO ? -ENOENT : r;
real_fops = debugfs_real_fops(filp);
+
+ r = debugfs_is_locked_down(inode, filp, real_fops);
+ if (r)
+ 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 95b5e78c22b1..a53a4748ebc1 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -23,6 +23,7 @@
#include <linux/parser.h>
#include <linux/magic.h>
#include <linux/slab.h>
+#include <linux/security.h>
#include "internal.h"
@@ -32,6 +33,32 @@ 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)
+{
+ int ret = security_locked_down(LOCKDOWN_DEBUGFS);
+
+ if (ret && (ia->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
+ return ret;
+ 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 +383,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);
@@ -516,7 +544,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) */
@@ -611,7 +639,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);
diff --git a/include/linux/security.h b/include/linux/security.h
index 53ea85889a48..097e4b0ce73f 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -94,6 +94,7 @@ enum lockdown_reason {
LOCKDOWN_TIOCSSERIAL,
LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_MMIOTRACE,
+ LOCKDOWN_DEBUGFS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index a7e75c614416..bbcb82985765 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -30,6 +30,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
[LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
+ [LOCKDOWN_DEBUGFS] = "debugfs access",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
--
2.22.0.410.gd8fdbe21b5-goog
Systems in lockdown mode should block the kexec of untrusted kernels.
For x86 and ARM we can ensure that a kernel is trustworthy by validating
a PE signature, but this isn't possible on other architectures. On those
platforms we can use IMA digital signatures instead. Add a function to
determine whether IMA has or will verify signatures for a given event type,
and if so permit kexec_file() even if the kernel is otherwise locked down.
This is restricted to cases where CONFIG_INTEGRITY_TRUSTED_KEYRING is set
in order to prevent an attacker from loading additional keys at runtime.
Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Mimi Zohar <[email protected]>
Cc: Dmitry Kasatkin <[email protected]>
Cc: [email protected]
---
include/linux/ima.h | 9 ++++++
kernel/kexec_file.c | 11 +++++--
security/integrity/ima/ima.h | 2 ++
security/integrity/ima/ima_main.c | 2 +-
security/integrity/ima/ima_policy.c | 50 +++++++++++++++++++++++++++++
5 files changed, 71 insertions(+), 3 deletions(-)
diff --git a/include/linux/ima.h b/include/linux/ima.h
index dc12fbcf484c..c30954acc660 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -132,4 +132,13 @@ static inline int ima_inode_removexattr(struct dentry *dentry,
return 0;
}
#endif /* CONFIG_IMA_APPRAISE */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+extern bool ima_appraise_signature(enum kernel_read_file_id func);
+#else
+static inline bool ima_appraise_signature(enum kernel_read_file_id func)
+{
+ return false;
+}
+#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
#endif /* _LINUX_IMA_H */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 27adb4312b03..539d0ca855bc 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -237,8 +237,15 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
goto out;
}
- ret = security_locked_down(LOCKDOWN_KEXEC);
- if (ret)
+ ret = 0;
+
+ /* If IMA is guaranteed to appraise a signature on the kexec
+ * image, permit it even if the kernel is otherwise locked
+ * down.
+ */
+ if (!ima_appraise_signature(READING_KEXEC_IMAGE) &&
+ security_locked_down(LOCKDOWN_KEXEC)) {
+ ret = -EPERM;
goto out;
break;
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index d213e835c498..3bc62062cfe8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -115,6 +115,8 @@ struct ima_kexec_hdr {
u64 count;
};
+extern const int read_idmap[];
+
#ifdef CONFIG_HAVE_IMA_KEXEC
void ima_load_kexec_buffer(void);
#else
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 357edd140c09..927fe889201a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -473,7 +473,7 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
return 0;
}
-static const int read_idmap[READING_MAX_ID] = {
+const int read_idmap[READING_MAX_ID] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_FIRMWARE_PREALLOC_BUFFER] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e0cc323f948f..8784449918e2 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1339,3 +1339,53 @@ int ima_policy_show(struct seq_file *m, void *v)
return 0;
}
#endif /* CONFIG_IMA_READ_POLICY */
+
+#if defined(CONFIG_IMA_APPRAISE) && defined(CONFIG_INTEGRITY_TRUSTED_KEYRING)
+/*
+ * ima_appraise_signature: whether IMA will appraise a given function using
+ * an IMA digital signature. This is restricted to cases where the kernel
+ * has a set of built-in trusted keys in order to avoid an attacker simply
+ * loading additional keys.
+ */
+bool ima_appraise_signature(enum kernel_read_file_id id)
+{
+ struct ima_rule_entry *entry;
+ bool found = false;
+ enum ima_hooks func;
+
+ if (id >= READING_MAX_ID)
+ return false;
+
+ func = read_idmap[id] ?: FILE_CHECK;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(entry, ima_rules, list) {
+ if (entry->action != APPRAISE)
+ continue;
+
+ /*
+ * A generic entry will match, but otherwise require that it
+ * match the func we're looking for
+ */
+ if (entry->func && entry->func != func)
+ continue;
+
+ /*
+ * We require this to be a digital signature, not a raw IMA
+ * hash.
+ */
+ if (entry->flags & IMA_DIGSIG_REQUIRED)
+ found = true;
+
+ /*
+ * We've found a rule that matches, so break now even if it
+ * didn't require a digital signature - a later rule that does
+ * won't override it, so would be a false positive.
+ */
+ break;
+ }
+
+ rcu_read_unlock();
+ return found;
+}
+#endif /* CONFIG_IMA_APPRAISE && CONFIG_INTEGRITY_TRUSTED_KEYRING */
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: Jiri Slaby <[email protected]>
Cc: [email protected]
---
drivers/tty/serial/serial_core.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 351843f847c0..a84f231a5df4 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -22,6 +22,7 @@
#include <linux/serial_core.h>
#include <linux/delay.h>
#include <linux/mutex.h>
+#include <linux/security.h>
#include <linux/irq.h>
#include <linux/uaccess.h>
@@ -852,6 +853,10 @@ 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;
+ retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
+ if (retval && (change_port || change_irq))
+ goto exit;
+
if (!capable(CAP_SYS_ADMIN)) {
retval = -EPERM;
if (change_irq || change_port ||
diff --git a/include/linux/security.h b/include/linux/security.h
index 03c125b277ca..61e3f4a62d16 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -91,6 +91,7 @@ enum lockdown_reason {
LOCKDOWN_MSR,
LOCKDOWN_ACPI_TABLES,
LOCKDOWN_PCMCIA_CIS,
+ LOCKDOWN_TIOCSSERIAL,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 7be3e8fb5847..c89046dc2155 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -27,6 +27,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
+ [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
From: David Howells <[email protected]>
Disallow the creation of perf and ftrace kprobes when the kernel is
locked down in confidentiality mode by preventing their registration.
This prevents kprobes from being used to access kernel memory to steal
crypto data, but continues to allow the use of kprobes from signed
modules.
Reported-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Naveen N. Rao <[email protected]>
Cc: Anil S Keshavamurthy <[email protected]>
Cc: [email protected]
Cc: Masami Hiramatsu <[email protected]>
---
include/linux/security.h | 1 +
kernel/trace/trace_kprobe.c | 5 +++++
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 3875f6df2ecc..e6e3e2403474 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -96,6 +96,7 @@ enum lockdown_reason {
LOCKDOWN_MMIOTRACE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
+ LOCKDOWN_KPROBES,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 5d5129b05df7..5a76a0f79d48 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -11,6 +11,7 @@
#include <linux/uaccess.h>
#include <linux/rculist.h>
#include <linux/error-injection.h>
+#include <linux/security.h>
#include "trace_dynevent.h"
#include "trace_kprobe_selftest.h"
@@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
{
int i, ret;
+ ret = security_locked_down(LOCKDOWN_KPROBES);
+ if (ret)
+ return ret;
+
if (trace_probe_is_registered(&tk->tp))
return -EINVAL;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 4c9b324dfc55..5a08c17f224d 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_MMIOTRACE] = "unsafe mmio",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
+ [LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
Tracefs may release more information about the kernel than desirable, so
restrict it when the kernel is locked down in confidentiality mode by
preventing open().
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Steven Rostedt <[email protected]>
---
fs/tracefs/inode.c | 43 +++++++++++++++++++++++++++++++++++-
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 44 insertions(+), 1 deletion(-)
diff --git a/fs/tracefs/inode.c b/fs/tracefs/inode.c
index 7098c49f3693..487d41f234f8 100644
--- a/fs/tracefs/inode.c
+++ b/fs/tracefs/inode.c
@@ -24,6 +24,7 @@
#include <linux/parser.h>
#include <linux/magic.h>
#include <linux/slab.h>
+#include <linux/security.h>
#define TRACEFS_DEFAULT_MODE 0700
@@ -31,6 +32,23 @@ static struct vfsmount *tracefs_mount;
static int tracefs_mount_count;
static bool tracefs_registered;
+static int default_open_file(struct inode *inode, struct file *filp)
+{
+ struct dentry *dentry = filp->f_path.dentry;
+ struct file_operations *real_fops;
+ int ret;
+
+ if (!dentry)
+ return -EINVAL;
+
+ ret = security_locked_down(LOCKDOWN_TRACEFS);
+ if (ret)
+ return ret;
+
+ real_fops = dentry->d_fsdata;
+ return real_fops->open(inode, filp);
+}
+
static ssize_t default_read_file(struct file *file, char __user *buf,
size_t count, loff_t *ppos)
{
@@ -50,6 +68,13 @@ static const struct file_operations tracefs_file_operations = {
.llseek = noop_llseek,
};
+static const struct file_operations tracefs_proxy_file_operations = {
+ .read = default_read_file,
+ .write = default_write_file,
+ .open = default_open_file,
+ .llseek = noop_llseek,
+};
+
static struct tracefs_dir_ops {
int (*mkdir)(const char *name);
int (*rmdir)(const char *name);
@@ -225,6 +250,12 @@ static int tracefs_apply_options(struct super_block *sb)
return 0;
}
+static void tracefs_destroy_inode(struct inode *inode)
+{
+ if (S_ISREG(inode->i_mode))
+ kfree(inode->i_fop);
+}
+
static int tracefs_remount(struct super_block *sb, int *flags, char *data)
{
int err;
@@ -260,6 +291,7 @@ static int tracefs_show_options(struct seq_file *m, struct dentry *root)
static const struct super_operations tracefs_super_operations = {
.statfs = simple_statfs,
+ .destroy_inode = tracefs_destroy_inode,
.remount_fs = tracefs_remount,
.show_options = tracefs_show_options,
};
@@ -393,6 +425,7 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
{
struct dentry *dentry;
struct inode *inode;
+ struct file_operations *proxy_fops;
if (!(mode & S_IFMT))
mode |= S_IFREG;
@@ -406,8 +439,16 @@ struct dentry *tracefs_create_file(const char *name, umode_t mode,
if (unlikely(!inode))
return failed_creating(dentry);
+ proxy_fops = kzalloc(sizeof(struct file_operations), GFP_KERNEL);
+ if (!proxy_fops)
+ return failed_creating(dentry);
+
+ dentry->d_fsdata = fops ? (void *)fops :
+ (void *)&tracefs_file_operations;
+ memcpy(proxy_fops, dentry->d_fsdata, sizeof(struct file_operations));
+ proxy_fops->open = default_open_file;
inode->i_mode = mode;
- inode->i_fop = fops ? fops : &tracefs_file_operations;
+ inode->i_fop = proxy_fops;
inode->i_private = data;
d_instantiate(dentry, inode);
fsnotify_create(dentry->d_parent->d_inode, dentry);
diff --git a/include/linux/security.h b/include/linux/security.h
index 097e4b0ce73f..438dc0892b96 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -100,6 +100,7 @@ enum lockdown_reason {
LOCKDOWN_KPROBES,
LOCKDOWN_BPF_READ,
LOCKDOWN_PERF,
+ LOCKDOWN_TRACEFS,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index bbcb82985765..98f9ee0026d5 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -36,6 +36,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_KPROBES] = "use of kprobes",
[LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
[LOCKDOWN_PERF] = "unsafe use of perf",
+ [LOCKDOWN_TRACEFS] = "use of tracefs",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
---
include/linux/security.h | 1 +
kernel/params.c | 27 ++++++++++++++++++++++-----
security/lockdown/lockdown.c | 1 +
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 61e3f4a62d16..88064d7f6827 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -92,6 +92,7 @@ enum lockdown_reason {
LOCKDOWN_ACPI_TABLES,
LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_TIOCSSERIAL,
+ LOCKDOWN_MODULE_PARAMETERS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/params.c b/kernel/params.c
index ce89f757e6da..f94fe79e331d 100644
--- a/kernel/params.c
+++ b/kernel/params.c
@@ -24,6 +24,7 @@
#include <linux/err.h>
#include <linux/slab.h>
#include <linux/ctype.h>
+#include <linux/security.h>
#ifdef CONFIG_SYSFS
/* Protects all built-in parameters, modules use their own param_lock */
@@ -108,13 +109,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 &&
+ security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
+ return false;
+ return true;
}
static int parse_one(char *param,
@@ -144,8 +151,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(¶ms[i]);
- err = params[i].ops->set(val, ¶ms[i]);
+ if (param_check_unsafe(¶ms[i], doing))
+ err = params[i].ops->set(val, ¶ms[i]);
+ else
+ err = -EPERM;
kernel_param_unlock(params[i].mod);
return err;
}
@@ -553,6 +562,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 +580,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;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index c89046dc2155..d03c4c296af7 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
[LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
+ [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
efivar_ssdt_load allows the kernel to import arbitrary ACPI code from an
EFI variable, which gives arbitrary code execution in ring 0. Prevent
that when the kernel is locked down.
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Ard Biesheuvel <[email protected]>
Cc: [email protected]
---
drivers/firmware/efi/efi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 55b77c576c42..9f92a013ab27 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -31,6 +31,7 @@
#include <linux/acpi.h>
#include <linux/ucs2_string.h>
#include <linux/memblock.h>
+#include <linux/security.h>
#include <asm/early_ioremap.h>
@@ -242,6 +243,11 @@ static void generic_ops_unregister(void)
static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata;
static int __init efivar_ssdt_setup(char *str)
{
+ int ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
+
+ if (ret)
+ return ret;
+
if (strlen(str) < sizeof(efivar_ssdt))
memcpy(efivar_ssdt, str, strlen(str));
else
--
2.22.0.410.gd8fdbe21b5-goog
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]>
cc: [email protected]
---
drivers/acpi/custom_method.c | 6 ++++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 8 insertions(+)
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index aa972dc5cb7e..6e56f9f43492 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -8,6 +8,7 @@
#include <linux/uaccess.h>
#include <linux/debugfs.h>
#include <linux/acpi.h>
+#include <linux/security.h>
#include "internal.h"
@@ -28,6 +29,11 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
struct acpi_table_header table;
acpi_status status;
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
+ if (ret)
+ return ret;
if (!(*ppos)) {
/* parse the table header to get the table length */
diff --git a/include/linux/security.h b/include/linux/security.h
index 30bc6f058926..cc2b5ee4cadd 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -89,6 +89,7 @@ enum lockdown_reason {
LOCKDOWN_PCI_ACCESS,
LOCKDOWN_IOPORT,
LOCKDOWN_MSR,
+ LOCKDOWN_ACPI_TABLES,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 297a065e6261..1725224f0024 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -25,6 +25,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_PCI_ACCESS] = "direct PCI access",
[LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_MSR] = "raw MSR access",
+ [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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. Disable them if the kernel has been locked down in
confidentiality mode.
Suggested-by: Alexei Starovoitov <[email protected]>
Signed-off-by: David Howells <[email protected]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [email protected]
cc: Chun-Yi Lee <[email protected]>
cc: Alexei Starovoitov <[email protected]>
Cc: Daniel Borkmann <[email protected]>
---
include/linux/security.h | 1 +
kernel/trace/bpf_trace.c | 20 +++++++++++++++++++-
security/lockdown/lockdown.c | 1 +
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index e6e3e2403474..de0d37b1fe79 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -97,6 +97,7 @@ enum lockdown_reason {
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
+ LOCKDOWN_BPF_READ,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d64c00afceb5..638f9b00a8df 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret)
+ return ret;
+
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
memset(dst, 0, size);
@@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
u32, size)
{
+ int ret;
+
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret)
+ return ret;
+
/*
* Ensure we're in user context which is safe for the helper to
* run. This helper has no business in a kthread.
@@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
int fmt_cnt = 0;
u64 unsafe_addr;
char buf[64];
- int i;
+ int i, ret;
+
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret)
+ return ret;
/*
* bpf_check()->check_func_arg()->check_stack_boundary()
@@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret)
+ return ret;
+
/*
* The strncpy_from_unsafe() call will likely not fill the entire
* buffer, but that's okay in this circumstance as we're probing
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 5a08c17f224d..2eea2cc13117 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
+ [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
---
drivers/pcmcia/cistpl.c | 5 +++++
include/linux/security.h | 1 +
security/lockdown/lockdown.c | 1 +
3 files changed, 7 insertions(+)
diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
index ac0672b8dfca..379c53610102 100644
--- a/drivers/pcmcia/cistpl.c
+++ b/drivers/pcmcia/cistpl.c
@@ -24,6 +24,7 @@
#include <linux/pci.h>
#include <linux/ioport.h>
#include <linux/io.h>
+#include <linux/security.h>
#include <asm/byteorder.h>
#include <asm/unaligned.h>
@@ -1578,6 +1579,10 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
struct pcmcia_socket *s;
int error;
+ error = security_locked_down(LOCKDOWN_PCMCIA_CIS);
+ if (error)
+ return error;
+
s = to_socket(container_of(kobj, struct device, kobj));
if (off)
diff --git a/include/linux/security.h b/include/linux/security.h
index cc2b5ee4cadd..03c125b277ca 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -90,6 +90,7 @@ enum lockdown_reason {
LOCKDOWN_IOPORT,
LOCKDOWN_MSR,
LOCKDOWN_ACPI_TABLES,
+ LOCKDOWN_PCMCIA_CIS,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 1725224f0024..7be3e8fb5847 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -26,6 +26,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_IOPORT] = "raw io port access",
[LOCKDOWN_MSR] = "raw MSR access",
[LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
+ [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
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 lockdown is enabled, 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]>
Signed-off-by: Matthew Garrett <[email protected]>
cc: [email protected]
---
drivers/acpi/tables.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 8fccbe49612a..41d9ccd0e075 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -34,6 +34,7 @@
#include <linux/memblock.h>
#include <linux/earlycpio.h>
#include <linux/initrd.h>
+#include <linux/security.h>
#include "internal.h"
#ifdef CONFIG_ACPI_CUSTOM_DSDT
@@ -539,6 +540,11 @@ void __init acpi_table_upgrade(void)
if (table_nr == 0)
return;
+ if (security_locked_down(LOCKDOWN_ACPI_TABLES)) {
+ 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.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Jiri Bohac <[email protected]>
cc: [email protected]
---
kernel/kexec_file.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index eec7e5bb2a08..27adb4312b03 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -237,7 +237,10 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
goto out;
}
- ret = 0;
+ ret = security_locked_down(LOCKDOWN_KEXEC);
+ if (ret)
+ goto out;
+
break;
/* All other errors are fatal, including nomem, unparseable
--
2.22.0.410.gd8fdbe21b5-goog
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]>
Signed-off-by: Matthew Garrett <[email protected]>
Cc: Jessica Yu <[email protected]>
---
include/linux/security.h | 1 +
kernel/module.c | 38 +++++++++++++++++++++++++++++-------
security/lockdown/lockdown.c | 1 +
3 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index c808d344ec75..46d85cd63b06 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -82,6 +82,7 @@ enum lsm_event {
*/
enum lockdown_reason {
LOCKDOWN_NONE,
+ LOCKDOWN_MODULE_SIGNATURE,
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/module.c b/kernel/module.c
index 0b9aa8ab89f0..6aa681edd660 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2763,8 +2763,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 ret, err = -ENODATA;
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+ const char *reason;
const void *mod = info->hdr;
/*
@@ -2779,16 +2780,39 @@ 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;
+ ret = security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
+ return ret;
+
+ /* 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)
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 8e39b36b8f33..25a3a5b0aa9c 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -18,6 +18,7 @@ static enum lockdown_reason kernel_locked_down;
static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_NONE] = "none",
+ [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.22.0.410.gd8fdbe21b5-goog
On Fri 2019-06-21 17:03:39, Matthew Garrett 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.
I keep getting these...
IIRC suse has patches to verify the images.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Fri, Jun 21, 2019 at 05:03:30PM -0700, Matthew Garrett wrote:
> The lockdown module is intended to allow for kernels to be locked down
> early in boot - sufficiently early that we don't have the ability to
> kmalloc() yet. Add support for early initialisation of some LSMs, and
> then add them to the list of names when we do full initialisation later.
> Early LSMs are initialised in link order and cannot be overridden via
> boot parameters, and cannot make use of kmalloc() (since the allocator
> isn't initialised yet).
>
> Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Kees Cook <[email protected]>
-Kees
> ---
> include/asm-generic/vmlinux.lds.h | 8 ++++-
> include/linux/lsm_hooks.h | 6 ++++
> include/linux/security.h | 1 +
> init/main.c | 1 +
> security/security.c | 50 ++++++++++++++++++++++++++-----
> 5 files changed, 57 insertions(+), 9 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index f8f6f04c4453..e1963352fdb6 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -208,8 +208,13 @@
> __start_lsm_info = .; \
> KEEP(*(.lsm_info.init)) \
> __end_lsm_info = .;
> +#define EARLY_LSM_TABLE() . = ALIGN(8); \
> + __start_early_lsm_info = .; \
> + KEEP(*(.early_lsm_info.init)) \
> + __end_early_lsm_info = .;
> #else
> #define LSM_TABLE()
> +#define EARLY_LSM_TABLE()
> #endif
>
> #define ___OF_TABLE(cfg, name) _OF_TABLE_##cfg(name)
> @@ -610,7 +615,8 @@
> ACPI_PROBE_TABLE(irqchip) \
> ACPI_PROBE_TABLE(timer) \
> EARLYCON_TABLE() \
> - LSM_TABLE()
> + LSM_TABLE() \
> + EARLY_LSM_TABLE()
>
> #define INIT_TEXT \
> *(.init.text .init.text.*) \
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index a240a3fc5fc4..66fd1eac7a32 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -2085,12 +2085,18 @@ struct lsm_info {
> };
>
> extern struct lsm_info __start_lsm_info[], __end_lsm_info[];
> +extern struct lsm_info __start_early_lsm_info[], __end_early_lsm_info[];
>
> #define DEFINE_LSM(lsm) \
> static struct lsm_info __lsm_##lsm \
> __used __section(.lsm_info.init) \
> __aligned(sizeof(unsigned long))
>
> +#define DEFINE_EARLY_LSM(lsm) \
> + static struct lsm_info __early_lsm_##lsm \
> + __used __section(.early_lsm_info.init) \
> + __aligned(sizeof(unsigned long))
> +
> #ifdef CONFIG_SECURITY_SELINUX_DISABLE
> /*
> * Assuring the safety of deleting a security module is up to
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 49f2685324b0..1bb6fb2f1523 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -194,6 +194,7 @@ int unregister_lsm_notifier(struct notifier_block *nb);
>
> /* prototypes */
> extern int security_init(void);
> +extern int early_security_init(void);
>
> /* Security operations */
> int security_binder_set_context_mgr(struct task_struct *mgr);
> diff --git a/init/main.c b/init/main.c
> index 598e278b46f7..f3faeb89c75f 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -563,6 +563,7 @@ asmlinkage __visible void __init start_kernel(void)
> boot_cpu_init();
> page_address_init();
> pr_notice("%s", linux_banner);
> + early_security_init();
> setup_arch(&command_line);
> /*
> * Set up the the initial canary and entropy after arch
> diff --git a/security/security.c b/security/security.c
> index 23cbb1a295a3..487e1f3eb2df 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -37,6 +37,7 @@
>
> /* How many LSMs were built into the kernel? */
> #define LSM_COUNT (__end_lsm_info - __start_lsm_info)
> +#define EARLY_LSM_COUNT (__end_early_lsm_info - __start_early_lsm_info)
>
> struct security_hook_heads security_hook_heads __lsm_ro_after_init;
> static ATOMIC_NOTIFIER_HEAD(lsm_notifier_chain);
> @@ -281,6 +282,8 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
> static void __init lsm_early_cred(struct cred *cred);
> static void __init lsm_early_task(struct task_struct *task);
>
> +static int lsm_append(const char *new, char **result);
> +
> static void __init ordered_lsm_init(void)
> {
> struct lsm_info **lsm;
> @@ -327,6 +330,26 @@ static void __init ordered_lsm_init(void)
> kfree(ordered_lsms);
> }
>
> +int __init early_security_init(void)
> +{
> + int i;
> + struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
> + struct lsm_info *lsm;
> +
> + for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
> + i++)
> + INIT_HLIST_HEAD(&list[i]);
> +
> + for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + if (!lsm->enabled)
> + lsm->enabled = &lsm_enabled_true;
> + prepare_lsm(lsm);
> + initialize_lsm(lsm);
> + }
> +
> + return 0;
> +}
> +
> /**
> * security_init - initializes the security framework
> *
> @@ -334,14 +357,18 @@ static void __init ordered_lsm_init(void)
> */
> int __init security_init(void)
> {
> - int i;
> - struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
> + struct lsm_info *lsm;
>
> pr_info("Security Framework initializing\n");
>
> - for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
> - i++)
> - INIT_HLIST_HEAD(&list[i]);
> + /*
> + * Append the names of the early LSM modules now that kmalloc() is
> + * available
> + */
> + for (lsm = __start_early_lsm_info; lsm < __end_early_lsm_info; lsm++) {
> + if (lsm->enabled)
> + lsm_append(lsm->name, &lsm_names);
> + }
>
> /* Load LSMs in specified order. */
> ordered_lsm_init();
> @@ -388,7 +415,7 @@ static bool match_last_lsm(const char *list, const char *lsm)
> return !strcmp(last, lsm);
> }
>
> -static int lsm_append(char *new, char **result)
> +static int lsm_append(const char *new, char **result)
> {
> char *cp;
>
> @@ -426,8 +453,15 @@ void __init security_add_hooks(struct security_hook_list *hooks, int count,
> hooks[i].lsm = lsm;
> hlist_add_tail_rcu(&hooks[i].list, hooks[i].head);
> }
> - if (lsm_append(lsm, &lsm_names) < 0)
> - panic("%s - Cannot get early memory.\n", __func__);
> +
> + /*
> + * Don't try to append during early_security_init(), we'll come back
> + * and fix this up afterwards.
> + */
> + if (slab_is_available()) {
> + if (lsm_append(lsm, &lsm_names) < 0)
> + panic("%s - Cannot get early memory.\n", __func__);
> + }
> }
>
> int call_lsm_notifier(enum lsm_event event, void *data)
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:31PM -0700, Matthew Garrett wrote:
> Add a mechanism to allow LSMs to make a policy decision around whether
> kernel functionality that would allow tampering with or examining the
> runtime state of the kernel should be permitted.
>
> Signed-off-by: Matthew Garrett <[email protected]>
Acked-by: Kees Cook <[email protected]>
-Kees
> ---
> include/linux/lsm_hooks.h | 2 ++
> include/linux/security.h | 11 +++++++++++
> security/security.c | 6 ++++++
> 3 files changed, 19 insertions(+)
>
> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
> index 66fd1eac7a32..df2aebc99838 100644
> --- a/include/linux/lsm_hooks.h
> +++ b/include/linux/lsm_hooks.h
> @@ -1790,6 +1790,7 @@ union security_list_options {
> int (*bpf_prog_alloc_security)(struct bpf_prog_aux *aux);
> void (*bpf_prog_free_security)(struct bpf_prog_aux *aux);
> #endif /* CONFIG_BPF_SYSCALL */
> + int (*locked_down)(enum lockdown_reason what);
> };
>
> struct security_hook_heads {
> @@ -2027,6 +2028,7 @@ struct security_hook_heads {
> struct hlist_head bpf_prog_alloc_security;
> struct hlist_head bpf_prog_free_security;
> #endif /* CONFIG_BPF_SYSCALL */
> + struct hlist_head locked_down;
> } __randomize_layout;
>
> /*
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1bb6fb2f1523..9eaf02e70707 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,12 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +enum lockdown_reason {
> + LOCKDOWN_NONE,
> + LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_CONFIDENTIALITY_MAX,
> +};
> +
> /* These functions are in security/commoncap.c */
> extern int cap_capable(const struct cred *cred, struct user_namespace *ns,
> int cap, unsigned int opts);
> @@ -389,6 +395,7 @@ void security_inode_invalidate_secctx(struct inode *inode);
> int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
> int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
> int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
> +int security_locked_down(enum lockdown_reason what);
> #else /* CONFIG_SECURITY */
>
> static inline int call_lsm_notifier(enum lsm_event event, void *data)
> @@ -1189,6 +1196,10 @@ static inline int security_inode_getsecctx(struct inode *inode, void **ctx, u32
> {
> return -EOPNOTSUPP;
> }
> +static inline int security_locked_down(enum lockdown_reason what)
> +{
> + return 0;
> +}
> #endif /* CONFIG_SECURITY */
>
> #ifdef CONFIG_SECURITY_NETWORK
> diff --git a/security/security.c b/security/security.c
> index 487e1f3eb2df..553f50e9a106 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2382,3 +2382,9 @@ void security_bpf_prog_free(struct bpf_prog_aux *aux)
> call_void_hook(bpf_prog_free_security, aux);
> }
> #endif /* CONFIG_BPF_SYSCALL */
> +
> +int security_locked_down(enum lockdown_reason what)
> +{
> + return call_int_hook(locked_down, 0, what);
> +}
> +EXPORT_SYMBOL(security_locked_down);
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:32PM -0700, Matthew Garrett wrote:
> While existing LSMs can be extended to handle lockdown policy,
> distributions generally want to be able to apply a straightforward
> static policy. This patch adds a simple LSM that can be configured to
> reject either integrity or all lockdown queries, and can be configured
> at runtime (through securityfs), boot time (via a kernel parameter) or
> build time (via a kconfig option). Based on initial code by David
> Howells.
>
> Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Cc: David Howells <[email protected]>
> ---
> .../admin-guide/kernel-parameters.txt | 9 +
> include/linux/security.h | 4 +
> security/Kconfig | 3 +-
> security/Makefile | 2 +
> security/lockdown/Kconfig | 47 +++++
> security/lockdown/Makefile | 1 +
> security/lockdown/lockdown.c | 172 ++++++++++++++++++
> 7 files changed, 237 insertions(+), 1 deletion(-)
> create mode 100644 security/lockdown/Kconfig
> create mode 100644 security/lockdown/Makefile
> create mode 100644 security/lockdown/lockdown.c
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2b8ee90bb644..fa336f6cd5bc 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2239,6 +2239,15 @@
> lockd.nlm_udpport=M [NFS] Assign UDP port.
> Format: <integer>
>
> + lockdown= [SECURITY]
> + { integrity | confidentiality }
> + Enable the kernel lockdown feature. If set to
> + integrity, kernel features that allow userland to
> + modify the running kernel are disabled. If set to
> + confidentiality, kernel features that allow userland
> + to extract confidential information from the kernel
> + are also disabled.
> +
> locktorture.nreaders_stress= [KNL]
> Set the number of locking read-acquisition kthreads.
> Defaults to being automatically set based on the
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 9eaf02e70707..c808d344ec75 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -76,6 +76,10 @@ enum lsm_event {
> LSM_POLICY_CHANGE,
> };
>
> +/*
> + * If you add to this, remember to extend lockdown_reasons in
> + * security/lockdown/lockdown.c.
> + */
> enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_INTEGRITY_MAX,
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463fb1450..c35aa72103df 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -236,12 +236,13 @@ source "security/apparmor/Kconfig"
> source "security/loadpin/Kconfig"
> source "security/yama/Kconfig"
> source "security/safesetid/Kconfig"
> +source "security/lockdown/Kconfig"
>
> source "security/integrity/Kconfig"
>
> config LSM
> string "Ordered list of enabled LSMs"
> - default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> + default "lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list will be ignored. This can be
> diff --git a/security/Makefile b/security/Makefile
> index c598b904938f..be1dd9d2cb2f 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -11,6 +11,7 @@ subdir-$(CONFIG_SECURITY_APPARMOR) += apparmor
> subdir-$(CONFIG_SECURITY_YAMA) += yama
> subdir-$(CONFIG_SECURITY_LOADPIN) += loadpin
> subdir-$(CONFIG_SECURITY_SAFESETID) += safesetid
> +subdir-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown
>
> # always enable default capabilities
> obj-y += commoncap.o
> @@ -27,6 +28,7 @@ obj-$(CONFIG_SECURITY_APPARMOR) += apparmor/
> obj-$(CONFIG_SECURITY_YAMA) += yama/
> obj-$(CONFIG_SECURITY_LOADPIN) += loadpin/
> obj-$(CONFIG_SECURITY_SAFESETID) += safesetid/
> +obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown/
> obj-$(CONFIG_CGROUP_DEVICE) += device_cgroup.o
>
> # Object integrity file lists
> diff --git a/security/lockdown/Kconfig b/security/lockdown/Kconfig
> new file mode 100644
> index 000000000000..7374ba76d8eb
> --- /dev/null
> +++ b/security/lockdown/Kconfig
> @@ -0,0 +1,47 @@
> +config SECURITY_LOCKDOWN_LSM
> + bool "Basic module for enforcing kernel lockdown"
> + depends on SECURITY
> + help
> + Build support for an LSM that enforces a coarse kernel lockdown
> + behaviour.
> +
> +config SECURITY_LOCKDOWN_LSM_EARLY
> + bool "Enable lockdown LSM early in init"
> + depends on SECURITY_LOCKDOWN_LSM
> + help
> + Enable the lockdown LSM early in boot. This is necessary in order
> + to ensure that lockdown enforcement can be carried out on kernel
> + boot parameters that are otherwise parsed before the security
> + subsystem is fully initialised. If enabled, lockdown will
> + unconditionally be called before any other LSMs.
> +
> +choice
> + prompt "Kernel default lockdown mode"
> + default LOCK_DOWN_KERNEL_FORCE_NONE
> + depends on SECURITY_LOCKDOWN_LSM
> + help
> + The kernel can be configured to default to differing levels of
> + lockdown.
> +
> +config LOCK_DOWN_KERNEL_FORCE_NONE
> + bool "None"
> + help
> + No lockdown functionality is enabled by default. Lockdown may be
> + enabled via the kernel commandline or /sys/kernel/security/lockdown.
> +
> +config LOCK_DOWN_KERNEL_FORCE_INTEGRITY
> + bool "Integrity"
> + help
> + The kernel runs in integrity mode by default. Features that allow
> + the kernel to be modified at runtime are disabled.
> +
> +config LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY
> + bool "Confidentiality"
> + help
> + The kernel runs in confidentiality mode by default. Features that
> + allow the kernel to be modified at runtime or that permit userland
> + code to read confidential material held inside the kernel are
> + disabled.
> +
> +endchoice
> +
> diff --git a/security/lockdown/Makefile b/security/lockdown/Makefile
> new file mode 100644
> index 000000000000..e3634b9017e7
> --- /dev/null
> +++ b/security/lockdown/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_SECURITY_LOCKDOWN_LSM) += lockdown.o
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> new file mode 100644
> index 000000000000..8e39b36b8f33
> --- /dev/null
> +++ b/security/lockdown/lockdown.c
> @@ -0,0 +1,172 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* 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>
> +#include <linux/lsm_hooks.h>
> +
> +static enum lockdown_reason kernel_locked_down;
> +
> +static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> + [LOCKDOWN_NONE] = "none",
> + [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> + [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> +};
> +
> +static enum lockdown_reason lockdown_levels[] = {LOCKDOWN_NONE,
> + LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_CONFIDENTIALITY_MAX};
> +
> +/*
> + * Put the kernel into lock-down mode.
> + */
> +static int lock_kernel_down(const char *where, enum lockdown_reason level)
> +{
> + if (kernel_locked_down >= level)
> + return -EPERM;
> +
> + kernel_locked_down = level;
> + pr_notice("Kernel is locked down from %s; see man kernel_lockdown.7\n",
> + where);
> + return 0;
> +}
> +
> +static int __init lockdown_param(char *level)
> +{
> + if (!level)
> + return -EINVAL;
> +
> + if (strcmp(level, "integrity") == 0)
> + lock_kernel_down("command line", LOCKDOWN_INTEGRITY_MAX);
> + else if (strcmp(level, "confidentiality") == 0)
> + lock_kernel_down("command line", LOCKDOWN_CONFIDENTIALITY_MAX);
> + else
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +early_param("lockdown", lockdown_param);
> +
> +/**
> + * lockdown_is_locked_down - Find out if the kernel is locked down
> + * @what: Tag to use in notice generated if lockdown is in effect
> + */
> +static int lockdown_is_locked_down(enum lockdown_reason what)
> +{
> + if ((kernel_locked_down >= what)) {
> + if (lockdown_reasons[what])
> + pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
> + lockdown_reasons[what]);
> + return -EPERM;
> + }
> +
> + return 0;
> +}
> +
> +static struct security_hook_list lockdown_hooks[] __lsm_ro_after_init = {
> + LSM_HOOK_INIT(locked_down, lockdown_is_locked_down),
> +};
> +
> +static int __init lockdown_lsm_init(void)
> +{
> +#if defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_INTEGRITY)
> + lock_kernel_down("Kernel configuration", LOCKDOWN_INTEGRITY_MAX);
> +#elif defined(CONFIG_LOCK_DOWN_KERNEL_FORCE_CONFIDENTIALITY)
> + lock_kernel_down("Kernel configuration", LOCKDOWN_CONFIDENTIALITY_MAX);
> +#endif
> + security_add_hooks(lockdown_hooks, ARRAY_SIZE(lockdown_hooks),
> + "lockdown");
> + return 0;
> +}
> +
> +static ssize_t lockdown_read(struct file *filp, char __user *buf, size_t count,
> + loff_t *ppos)
> +{
> + char temp[80];
> + int i, offset=0;
> +
> + for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
> + enum lockdown_reason level = lockdown_levels[i];
> +
> + if (lockdown_reasons[level]) {
> + const char *label = lockdown_reasons[level];
> +
> + if (kernel_locked_down == level)
> + offset += sprintf(temp+offset, "[%s] ", label);
> + else
> + offset += sprintf(temp+offset, "%s ", label);
> + }
> + }
> +
> + /* Convert the last space to a newline if needed. */
> + if (offset > 0)
> + temp[offset-1] = '\n';
> +
> + return simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
> +}
> +
> +static ssize_t lockdown_write(struct file *file, const char __user *buf,
> + size_t n, loff_t *ppos)
> +{
> + char *state;
> + int i, len, err = -EINVAL;
> +
> + state = memdup_user_nul(buf, n);
> + if (IS_ERR(state))
> + return PTR_ERR(state);
> +
> + len = strlen(state);
> + if (len && state[len-1] == '\n') {
> + state[len-1] = '\0';
> + len--;
> + }
> +
> + for (i = 0; i < ARRAY_SIZE(lockdown_levels); i++) {
> + enum lockdown_reason level = lockdown_levels[i];
> + const char *label = lockdown_reasons[level];
> +
> + if (label && !strcmp(state, label))
> + err = lock_kernel_down("securityfs", level);
> + }
> +
> + kfree(state);
> + return err ? err : n;
> +}
> +
> +static const struct file_operations lockdown_ops = {
> + .read = lockdown_read,
> + .write = lockdown_write,
> +};
> +
> +static int __init lockdown_secfs_init(void)
> +{
> + struct dentry *dentry;
> +
> + dentry = securityfs_create_file("lockdown", 0600, NULL, NULL,
> + &lockdown_ops);
> + if (IS_ERR(dentry))
> + return PTR_ERR(dentry);
> +
> + return 0;
> +}
> +
> +core_initcall(lockdown_secfs_init);
> +
> +#ifdef CONFIG_SECURITY_LOCKDOWN_LSM_EARLY
> +DEFINE_EARLY_LSM(lockdown) = {
> +#else
> +DEFINE_LSM(lockdown) = {
> +#endif
> + .name = "lockdown",
> + .init = lockdown_lsm_init,
> +};
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:33PM -0700, 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.
>
> 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]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: Jessica Yu <[email protected]>
> ---
> include/linux/security.h | 1 +
> kernel/module.c | 38 +++++++++++++++++++++++++++++-------
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c808d344ec75..46d85cd63b06 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -82,6 +82,7 @@ enum lsm_event {
> */
> enum lockdown_reason {
> LOCKDOWN_NONE,
> + LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/module.c b/kernel/module.c
> index 0b9aa8ab89f0..6aa681edd660 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2763,8 +2763,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 ret, err = -ENODATA;
> const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
> + const char *reason;
> const void *mod = info->hdr;
>
> /*
> @@ -2779,16 +2780,39 @@ 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;
> + ret = security_locked_down(LOCKDOWN_MODULE_SIGNATURE);
> + return ret;
return security_locked_down(LOCKDOWN_MODULE_SIGNATURE); ? Means no need
to add "ret". Regardless:
Reviewed-by: Kees Cook <[email protected]>
-Kees
> +
> + /* 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)
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 8e39b36b8f33..25a3a5b0aa9c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -18,6 +18,7 @@ static enum lockdown_reason kernel_locked_down;
>
> static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> + [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:34PM -0700, Matthew Garrett wrote:
> 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: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: [email protected]
> ---
> drivers/char/mem.c | 6 +++++-
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index b08dc50f9f26..93c02493f0fa 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -29,8 +29,8 @@
> #include <linux/export.h>
> #include <linux/io.h>
> #include <linux/uio.h>
> -
> #include <linux/uaccess.h>
> +#include <linux/security.h>
>
> #ifdef CONFIG_IA64
> # include <linux/efi.h>
> @@ -786,6 +786,10 @@ static loff_t memory_lseek(struct file *file, loff_t offset, int orig)
>
> static int open_port(struct inode *inode, struct file *filp)
> {
> + int ret = security_locked_down(LOCKDOWN_DEV_MEM);
> +
> + if (ret)
> + return ret;
> return capable(CAP_SYS_RAWIO) ? 0 : -EPERM;
Usually the ordering for LSM tests tends to follow capable checks, which
allows for things like audit to generate logs for capability rejections,
etc. I'd expect this to be:
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
return security_locked_down(LOCKDOWN_DEV_MEM)
With that fixed:
Reviewed-by: Kees Cook <[email protected]>
-Kees
> }
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 46d85cd63b06..200175c8605a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -83,6 +83,7 @@ enum lsm_event {
> enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_MODULE_SIGNATURE,
> + LOCKDOWN_DEV_MEM,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 25a3a5b0aa9c..565c87451f0f 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -19,6 +19,7 @@ static enum lockdown_reason kernel_locked_down;
> static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> + [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:35PM -0700, Matthew Garrett wrote:
> 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: David Howells <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[email protected]>
> Acked-by: Dave Young <[email protected]>
> cc: [email protected]
> ---
> include/linux/security.h | 1 +
> kernel/kexec.c | 8 ++++++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 10 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 200175c8605a..00a31ab2e5ba 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -84,6 +84,7 @@ enum lockdown_reason {
> LOCKDOWN_NONE,
> LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_DEV_MEM,
> + LOCKDOWN_KEXEC,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/kexec.c b/kernel/kexec.c
> index 68559808fdfa..ec3f07a4b1c0 100644
> --- a/kernel/kexec.c
> +++ b/kernel/kexec.c
> @@ -207,6 +207,14 @@ 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
> + */
> + result = security_locked_down(LOCKDOWN_KEXEC);
> + if (result)
> + return result;
> +
> /*
> * Verify we have a legal set of flags
> * This leaves us room for future extensions.
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 565c87451f0f..08fcd8116db3 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -20,6 +20,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_NONE] = "none",
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> + [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:36PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: [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 22f60dd26460..4243359ac509 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -182,6 +182,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.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:38PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Reviewed-by: Jiri Bohac <[email protected]>
> cc: [email protected]
> ---
> kernel/kexec_file.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index eec7e5bb2a08..27adb4312b03 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -237,7 +237,10 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
> goto out;
> }
>
> - ret = 0;
> + ret = security_locked_down(LOCKDOWN_KEXEC);
> + if (ret)
> + goto out;
> +
> break;
>
> /* All other errors are fatal, including nomem, unparseable
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:39PM -0700, Matthew Garrett 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.
>
> Signed-off-by: Josh Boyer <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> cc: [email protected]
> ---
> include/linux/security.h | 1 +
> kernel/power/hibernate.c | 3 ++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 00a31ab2e5ba..a051f21a1144 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -85,6 +85,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_SIGNATURE,
> LOCKDOWN_DEV_MEM,
> LOCKDOWN_KEXEC,
> + LOCKDOWN_HIBERNATION,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c
> index abef759de7c8..3a9cb2d3da4a 100644
> --- a/kernel/power/hibernate.c
> +++ b/kernel/power/hibernate.c
> @@ -32,6 +32,7 @@
> #include <linux/ctype.h>
> #include <linux/genhd.h>
> #include <linux/ktime.h>
> +#include <linux/security.h>
> #include <trace/events/power.h>
>
> #include "power.h"
> @@ -70,7 +71,7 @@ static const struct platform_hibernation_ops *hibernation_ops;
>
> bool hibernation_available(void)
> {
> - return (nohibernate == 0);
> + return nohibernate == 0 && !security_locked_down(LOCKDOWN_HIBERNATION);
> }
>
> /**
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 08fcd8116db3..ce5b3da9bd09 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -21,6 +21,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_SIGNATURE] = "unsigned module loading",
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> + [LOCKDOWN_HIBERNATION] = "hibernation",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:40PM -0700, Matthew Garrett wrote:
> 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: David Howells <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[email protected]>
> Acked-by: Bjorn Helgaas <[email protected]>
> cc: [email protected]
> ---
> drivers/pci/pci-sysfs.c | 16 ++++++++++++++++
> drivers/pci/proc.c | 14 ++++++++++++--
> drivers/pci/syscall.c | 4 +++-
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 5 files changed, 33 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 25794c27c7a4..e1011efb5a31 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -903,6 +903,11 @@ static ssize_t pci_write_config(struct file *filp, struct kobject *kobj,
> unsigned int size = count;
> loff_t init_off = off;
> u8 *data = (u8 *) buf;
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (off > dev->cfg_size)
> return 0;
> @@ -1165,6 +1170,11 @@ static int pci_mmap_resource(struct kobject *kobj, struct bin_attribute *attr,
> int bar = (unsigned long)attr->private;
> enum pci_mmap_state mmap_type;
> struct resource *res = &pdev->resource[bar];
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
> return -EINVAL;
> @@ -1241,6 +1251,12 @@ 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)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
> +
> 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..a72258d70407 100644
> --- a/drivers/pci/proc.c
> +++ b/drivers/pci/proc.c
> @@ -13,6 +13,7 @@
> #include <linux/seq_file.h>
> #include <linux/capability.h>
> #include <linux/uaccess.h>
> +#include <linux/security.h>
> #include <asm/byteorder.h>
> #include "pci.h"
>
> @@ -115,7 +116,11 @@ static ssize_t proc_bus_pci_write(struct file *file, const char __user *buf,
> struct pci_dev *dev = PDE_DATA(ino);
> int pos = *ppos;
> int size = dev->cfg_size;
> - int cnt;
> + int cnt, ret;
> +
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
>
> if (pos >= size)
> return 0;
> @@ -196,6 +201,10 @@ static long proc_bus_pci_ioctl(struct file *file, unsigned int cmd,
> #endif /* HAVE_PCI_MMAP */
> int ret = 0;
>
> + ret = security_locked_down(LOCKDOWN_PCI_ACCESS);
> + if (ret)
> + return ret;
> +
> switch (cmd) {
> case PCIIOC_CONTROLLER:
> ret = pci_domain_nr(dev->bus);
> @@ -237,7 +246,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) ||
> + security_locked_down(LOCKDOWN_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..31e39558d49d 100644
> --- a/drivers/pci/syscall.c
> +++ b/drivers/pci/syscall.c
> @@ -7,6 +7,7 @@
>
> #include <linux/errno.h>
> #include <linux/pci.h>
> +#include <linux/security.h>
> #include <linux/syscalls.h>
> #include <linux/uaccess.h>
> #include "pci.h"
> @@ -90,7 +91,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) ||
> + security_locked_down(LOCKDOWN_PCI_ACCESS))
> return -EPERM;
>
> dev = pci_get_domain_bus_and_slot(0, bus, dfn);
> diff --git a/include/linux/security.h b/include/linux/security.h
> index a051f21a1144..1b849f10dec6 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -86,6 +86,7 @@ enum lockdown_reason {
> LOCKDOWN_DEV_MEM,
> LOCKDOWN_KEXEC,
> LOCKDOWN_HIBERNATION,
> + LOCKDOWN_PCI_ACCESS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index ce5b3da9bd09..e2ee8a16b94c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -22,6 +22,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_DEV_MEM] = "/dev/mem,kmem,port",
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_HIBERNATION] = "hibernation",
> + [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:41PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
> arch/x86/kernel/ioport.c | 7 +++++--
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/ioport.c b/arch/x86/kernel/ioport.c
> index 0fe1c8782208..61a89d3c0382 100644
> --- a/arch/x86/kernel/ioport.c
> +++ b/arch/x86/kernel/ioport.c
> @@ -11,6 +11,7 @@
> #include <linux/errno.h>
> #include <linux/types.h>
> #include <linux/ioport.h>
> +#include <linux/security.h>
> #include <linux/smp.h>
> #include <linux/stddef.h>
> #include <linux/slab.h>
> @@ -31,7 +32,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) ||
> + security_locked_down(LOCKDOWN_IOPORT)))
> return -EPERM;
>
> /*
> @@ -126,7 +128,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) ||
> + security_locked_down(LOCKDOWN_IOPORT))
> return -EPERM;
> }
> regs->flags = (regs->flags & ~X86_EFLAGS_IOPL) |
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1b849f10dec6..60569b7e9465 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -87,6 +87,7 @@ enum lockdown_reason {
> LOCKDOWN_KEXEC,
> LOCKDOWN_HIBERNATION,
> LOCKDOWN_PCI_ACCESS,
> + LOCKDOWN_IOPORT,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index e2ee8a16b94c..895ef3ba1b4c 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -23,6 +23,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_KEXEC] = "kexec of unsigned images",
> [LOCKDOWN_HIBERNATION] = "hibernation",
> [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> + [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:43PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> cc: [email protected]
> ---
> drivers/acpi/custom_method.c | 6 ++++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 8 insertions(+)
>
> diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
> index aa972dc5cb7e..6e56f9f43492 100644
> --- a/drivers/acpi/custom_method.c
> +++ b/drivers/acpi/custom_method.c
> @@ -8,6 +8,7 @@
> #include <linux/uaccess.h>
> #include <linux/debugfs.h>
> #include <linux/acpi.h>
> +#include <linux/security.h>
>
> #include "internal.h"
>
> @@ -28,6 +29,11 @@ static ssize_t cm_write(struct file *file, const char __user * user_buf,
>
> struct acpi_table_header table;
> acpi_status status;
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
> + if (ret)
> + return ret;
>
> if (!(*ppos)) {
> /* parse the table header to get the table length */
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 30bc6f058926..cc2b5ee4cadd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -89,6 +89,7 @@ enum lockdown_reason {
> LOCKDOWN_PCI_ACCESS,
> LOCKDOWN_IOPORT,
> LOCKDOWN_MSR,
> + LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 297a065e6261..1725224f0024 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -25,6 +25,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_PCI_ACCESS] = "direct PCI access",
> [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_MSR] = "raw MSR access",
> + [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:44PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: Dave Young <[email protected]>
> cc: [email protected]
> ---
> drivers/acpi/osl.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
> index f29e427d0d1d..60cda8a0f36b 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -40,6 +40,7 @@
> #include <linux/list.h>
> #include <linux/jiffies.h>
> #include <linux/semaphore.h>
> +#include <linux/security.h>
>
> #include <asm/io.h>
> #include <linux/uaccess.h>
> @@ -194,7 +195,7 @@ acpi_physical_address __init acpi_os_get_root_pointer(void)
> acpi_physical_address pa;
>
> #ifdef CONFIG_KEXEC
> - if (acpi_rsdp)
> + if (acpi_rsdp && !security_locked_down(LOCKDOWN_ACPI_TABLES))
> return acpi_rsdp;
> #endif
> pa = acpi_arch_get_root_pointer();
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:45PM -0700, Matthew Garrett wrote:
> 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 lockdown is enabled, 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: [email protected]
> ---
> drivers/acpi/tables.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 8fccbe49612a..41d9ccd0e075 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -34,6 +34,7 @@
> #include <linux/memblock.h>
> #include <linux/earlycpio.h>
> #include <linux/initrd.h>
> +#include <linux/security.h>
> #include "internal.h"
>
> #ifdef CONFIG_ACPI_CUSTOM_DSDT
> @@ -539,6 +540,11 @@ void __init acpi_table_upgrade(void)
> if (table_nr == 0)
> return;
>
> + if (security_locked_down(LOCKDOWN_ACPI_TABLES)) {
> + 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.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:47PM -0700, Matthew Garrett wrote:
> 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]>
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: Jiri Slaby <[email protected]>
> Cc: [email protected]
> ---
> drivers/tty/serial/serial_core.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 351843f847c0..a84f231a5df4 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -22,6 +22,7 @@
> #include <linux/serial_core.h>
> #include <linux/delay.h>
> #include <linux/mutex.h>
> +#include <linux/security.h>
>
> #include <linux/irq.h>
> #include <linux/uaccess.h>
> @@ -852,6 +853,10 @@ 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;
>
> + retval = security_locked_down(LOCKDOWN_TIOCSSERIAL);
> + if (retval && (change_port || change_irq))
> + goto exit;
> +
> if (!capable(CAP_SYS_ADMIN)) {
> retval = -EPERM;
> if (change_irq || change_port ||
This should be moved after the capable test. With that fixed:
Reviewed-by: Kees Cook <[email protected]>
-Kees
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 03c125b277ca..61e3f4a62d16 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -91,6 +91,7 @@ enum lockdown_reason {
> LOCKDOWN_MSR,
> LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_PCMCIA_CIS,
> + LOCKDOWN_TIOCSSERIAL,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 7be3e8fb5847..c89046dc2155 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -27,6 +27,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MSR] = "raw MSR access",
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> + [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:46PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> drivers/pcmcia/cistpl.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/drivers/pcmcia/cistpl.c b/drivers/pcmcia/cistpl.c
> index ac0672b8dfca..379c53610102 100644
> --- a/drivers/pcmcia/cistpl.c
> +++ b/drivers/pcmcia/cistpl.c
> @@ -24,6 +24,7 @@
> #include <linux/pci.h>
> #include <linux/ioport.h>
> #include <linux/io.h>
> +#include <linux/security.h>
> #include <asm/byteorder.h>
> #include <asm/unaligned.h>
>
> @@ -1578,6 +1579,10 @@ static ssize_t pccard_store_cis(struct file *filp, struct kobject *kobj,
> struct pcmcia_socket *s;
> int error;
>
> + error = security_locked_down(LOCKDOWN_PCMCIA_CIS);
> + if (error)
> + return error;
> +
> s = to_socket(container_of(kobj, struct device, kobj));
>
> if (off)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index cc2b5ee4cadd..03c125b277ca 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -90,6 +90,7 @@ enum lockdown_reason {
> LOCKDOWN_IOPORT,
> LOCKDOWN_MSR,
> LOCKDOWN_ACPI_TABLES,
> + LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 1725224f0024..7be3e8fb5847 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -26,6 +26,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_IOPORT] = "raw io port access",
> [LOCKDOWN_MSR] = "raw MSR access",
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> + [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:48PM -0700, Matthew Garrett wrote:
> 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]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> include/linux/security.h | 1 +
> kernel/params.c | 27 ++++++++++++++++++++++-----
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 61e3f4a62d16..88064d7f6827 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -92,6 +92,7 @@ enum lockdown_reason {
> LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> + LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/params.c b/kernel/params.c
> index ce89f757e6da..f94fe79e331d 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -24,6 +24,7 @@
> #include <linux/err.h>
> #include <linux/slab.h>
> #include <linux/ctype.h>
> +#include <linux/security.h>
>
> #ifdef CONFIG_SYSFS
> /* Protects all built-in parameters, modules use their own param_lock */
> @@ -108,13 +109,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 &&
> + security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
> + return false;
> + return true;
> }
>
> static int parse_one(char *param,
> @@ -144,8 +151,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(¶ms[i]);
> - err = params[i].ops->set(val, ¶ms[i]);
> + if (param_check_unsafe(¶ms[i], doing))
> + err = params[i].ops->set(val, ¶ms[i]);
> + else
> + err = -EPERM;
> kernel_param_unlock(params[i].mod);
> return err;
> }
> @@ -553,6 +562,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 +580,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;
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index c89046dc2155..d03c4c296af7 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> + [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:49PM -0700, Matthew Garrett wrote:
> 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. This is
> a runtime check rather than buildtime in order to allow configurations
> where the same kernel may be run in both locked down or permissive modes
> depending on local policy.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: David Howells <[email protected]
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[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]
> ---
> arch/x86/mm/testmmiotrace.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/arch/x86/mm/testmmiotrace.c b/arch/x86/mm/testmmiotrace.c
> index f6ae6830b341..6b9486baa2e9 100644
> --- a/arch/x86/mm/testmmiotrace.c
> +++ b/arch/x86/mm/testmmiotrace.c
> @@ -7,6 +7,7 @@
> #include <linux/module.h>
> #include <linux/io.h>
> #include <linux/mmiotrace.h>
> +#include <linux/security.h>
>
> static unsigned long mmio_address;
> module_param_hw(mmio_address, ulong, iomem, 0);
> @@ -114,6 +115,10 @@ static void do_test_bulk_ioremapping(void)
> static int __init init(void)
> {
> unsigned long size = (read_far) ? (8 << 20) : (16 << 10);
> + int ret = security_locked_down(LOCKDOWN_MMIOTRACE);
> +
> + if (ret)
> + return ret;
>
> if (mmio_address == 0) {
> pr_err("you have to use the module argument mmio_address.\n");
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 88064d7f6827..c649cb91e762 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -93,6 +93,7 @@ enum lockdown_reason {
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> LOCKDOWN_MODULE_PARAMETERS,
> + LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index d03c4c296af7..cd86ed9f4d4b 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -29,6 +29,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> + [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:50PM -0700, Matthew Garrett wrote:
> From: David Howells <[email protected]>
>
> Disallow access to /proc/kcore when the kernel is locked down to prevent
> access to cryptographic data. This is limited to lockdown
> confidentiality mode and is still permitted in integrity mode.
>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> ---
> fs/proc/kcore.c | 5 +++++
> include/linux/security.h | 1 +
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/fs/proc/kcore.c b/fs/proc/kcore.c
> index d29d869abec1..4e95edb1e282 100644
> --- a/fs/proc/kcore.c
> +++ b/fs/proc/kcore.c
> @@ -31,6 +31,7 @@
> #include <linux/ioport.h>
> #include <linux/memory.h>
> #include <linux/sched/task.h>
> +#include <linux/security.h>
> #include <asm/sections.h>
> #include "internal.h"
>
> @@ -545,6 +546,10 @@ read_kcore(struct file *file, char __user *buffer, size_t buflen, loff_t *fpos)
>
> static int open_kcore(struct inode *inode, struct file *filp)
> {
> + int ret = security_locked_down(LOCKDOWN_KCORE);
> +
> + if (ret)
> + return ret;
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
Another capable() check ordering fix needed. With that:
Reviewed-by: Kees Cook <[email protected]>
-Kees
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index c649cb91e762..3875f6df2ecc 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -95,6 +95,7 @@ enum lockdown_reason {
> LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> + LOCKDOWN_KCORE,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index cd86ed9f4d4b..4c9b324dfc55 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -31,6 +31,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> + [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:51PM -0700, Matthew Garrett wrote:
> From: David Howells <[email protected]>
>
> Disallow the creation of perf and ftrace kprobes when the kernel is
> locked down in confidentiality mode by preventing their registration.
> This prevents kprobes from being used to access kernel memory to steal
> crypto data, but continues to allow the use of kprobes from signed
> modules.
>
> Reported-by: Alexei Starovoitov <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Anil S Keshavamurthy <[email protected]>
> Cc: [email protected]
> Cc: Masami Hiramatsu <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> ---
> include/linux/security.h | 1 +
> kernel/trace/trace_kprobe.c | 5 +++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3875f6df2ecc..e6e3e2403474 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -96,6 +96,7 @@ enum lockdown_reason {
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> + LOCKDOWN_KPROBES,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5d5129b05df7..5a76a0f79d48 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -11,6 +11,7 @@
> #include <linux/uaccess.h>
> #include <linux/rculist.h>
> #include <linux/error-injection.h>
> +#include <linux/security.h>
>
> #include "trace_dynevent.h"
> #include "trace_kprobe_selftest.h"
> @@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> int i, ret;
>
> + ret = security_locked_down(LOCKDOWN_KPROBES);
> + if (ret)
> + return ret;
> +
> if (trace_probe_is_registered(&tk->tp))
> return -EINVAL;
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4c9b324dfc55..5a08c17f224d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> + [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:52PM -0700, Matthew Garrett wrote:
> 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. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <[email protected]>
> Signed-off-by: David Howells <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: [email protected]
> cc: Chun-Yi Lee <[email protected]>
> cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
> ---
> include/linux/security.h | 1 +
> kernel/trace/bpf_trace.c | 20 +++++++++++++++++++-
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index e6e3e2403474..de0d37b1fe79 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -97,6 +97,7 @@ enum lockdown_reason {
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> + LOCKDOWN_BPF_READ,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:53PM -0700, Matthew Garrett wrote:
> 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]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Arnaldo Carvalho de Melo <[email protected]>
> ---
> include/linux/security.h | 1 +
> kernel/events/core.c | 7 +++++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 9 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index de0d37b1fe79..53ea85889a48 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -98,6 +98,7 @@ enum lockdown_reason {
> LOCKDOWN_KCORE,
> LOCKDOWN_KPROBES,
> LOCKDOWN_BPF_READ,
> + LOCKDOWN_PERF,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 72d06e302e99..77f36551756e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -10731,6 +10731,13 @@ SYSCALL_DEFINE5(perf_event_open,
> return -EINVAL;
> }
>
> + err = security_locked_down(LOCKDOWN_PERF);
> + if (err && (attr.sample_type & PERF_SAMPLE_REGS_INTR))
> + /* REGS_INTR can leak data, lockdown must prevent this */
> + return err;
> + else
> + err = 0;
> +
> /* Only privileged users can get physical addresses */
> if ((attr.sample_type & PERF_SAMPLE_PHYS_ADDR) &&
> perf_paranoid_kernel() && !capable(CAP_SYS_ADMIN))
With moar capable() ordering fixed...
Reviewed-by: Kees Cook <[email protected]>
-Kees
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 2eea2cc13117..a7e75c614416 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -34,6 +34,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> + [LOCKDOWN_PERF] = "unsafe use of perf",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:57PM -0700, Matthew Garrett wrote:
> efivar_ssdt_load allows the kernel to import arbitrary ACPI code from an
> EFI variable, which gives arbitrary code execution in ring 0. Prevent
> that when the kernel is locked down.
>
> Signed-off-by: Matthew Garrett <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
-Kees
> Cc: Ard Biesheuvel <[email protected]>
> Cc: [email protected]
> ---
> drivers/firmware/efi/efi.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 55b77c576c42..9f92a013ab27 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -31,6 +31,7 @@
> #include <linux/acpi.h>
> #include <linux/ucs2_string.h>
> #include <linux/memblock.h>
> +#include <linux/security.h>
>
> #include <asm/early_ioremap.h>
>
> @@ -242,6 +243,11 @@ static void generic_ops_unregister(void)
> static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata;
> static int __init efivar_ssdt_setup(char *str)
> {
> + int ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
> +
> + if (ret)
> + return ret;
> +
> if (strlen(str) < sizeof(efivar_ssdt))
> memcpy(efivar_ssdt, str, strlen(str));
> else
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, Jun 21, 2019 at 05:03:58PM -0700, Matthew Garrett wrote:
> 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/lockdown/lockdown.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 98f9ee0026d5..9ca6f442fbc7 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -83,8 +83,8 @@ static int lockdown_is_locked_down(enum lockdown_reason what)
> {
> if ((kernel_locked_down >= what)) {
To satisfy my paranoia, can you just add here:
if (WARN(what > LOCKDOWN_..._MAX))
return -EPERM;
With that:
Reviewed-by: Kees Cook <[email protected]>
-Kees
> if (lockdown_reasons[what])
> - pr_notice("Lockdown: %s is restricted; see man kernel_lockdown.7\n",
> - lockdown_reasons[what]);
> + pr_notice("Lockdown: %s: %s is restricted; see man kernel_lockdown.7\n",
> + current->comm, lockdown_reasons[what]);
> return -EPERM;
> }
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Kees Cook
On Fri, 21 Jun 2019 17:03:51 -0700
Matthew Garrett <[email protected]> wrote:
> From: David Howells <[email protected]>
>
> Disallow the creation of perf and ftrace kprobes when the kernel is
> locked down in confidentiality mode by preventing their registration.
> This prevents kprobes from being used to access kernel memory to steal
> crypto data, but continues to allow the use of kprobes from signed
> modules.
Looks (and sounds) good to me.
Acked-by: Masami Hiramatsu <[email protected]>
Thank you,
>
> Reported-by: Alexei Starovoitov <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: Naveen N. Rao <[email protected]>
> Cc: Anil S Keshavamurthy <[email protected]>
> Cc: [email protected]
> Cc: Masami Hiramatsu <[email protected]>
> ---
> include/linux/security.h | 1 +
> kernel/trace/trace_kprobe.c | 5 +++++
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 7 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 3875f6df2ecc..e6e3e2403474 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -96,6 +96,7 @@ enum lockdown_reason {
> LOCKDOWN_MMIOTRACE,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_KCORE,
> + LOCKDOWN_KPROBES,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
>
> diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
> index 5d5129b05df7..5a76a0f79d48 100644
> --- a/kernel/trace/trace_kprobe.c
> +++ b/kernel/trace/trace_kprobe.c
> @@ -11,6 +11,7 @@
> #include <linux/uaccess.h>
> #include <linux/rculist.h>
> #include <linux/error-injection.h>
> +#include <linux/security.h>
>
> #include "trace_dynevent.h"
> #include "trace_kprobe_selftest.h"
> @@ -415,6 +416,10 @@ static int __register_trace_kprobe(struct trace_kprobe *tk)
> {
> int i, ret;
>
> + ret = security_locked_down(LOCKDOWN_KPROBES);
> + if (ret)
> + return ret;
> +
> if (trace_probe_is_registered(&tk->tp))
> return -EINVAL;
>
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 4c9b324dfc55..5a08c17f224d 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -32,6 +32,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_MMIOTRACE] = "unsafe mmio",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> + [LOCKDOWN_KPROBES] = "use of kprobes",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
--
Masami Hiramatsu <[email protected]>
On Fri, 21 Jun 2019, Matthew Garrett wrote:
> 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. This is
> a runtime check rather than buildtime in order to allow configurations
> where the same kernel may be run in both locked down or permissive modes
> depending on local policy.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Signed-off-by: David Howells <[email protected]
> Signed-off-by: Matthew Garrett <[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]
Reviewed-by: Thomas Gleixner <[email protected]>
On 06/21/19 at 05:03pm, Matthew Garrett wrote:
> 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 if there is a signature for which we
> have a key, but signature doesn't match - even if in non-forcing mode.
>
> (3) kexec fails with EBADMSG or some other error if there is a signature
> which cannot be parsed - even if in non-forcing mode.
>
> (4) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
> the signature - even if in non-forcing mode.
>
> ]
Seems I do not see EBADMSG and ELIBBAD in this patch, also kexec fails
with proper errno instead of EKEYREJECTED only.
I may missed something? Other than the patch log issue:
Reviewed-by: Dave Young <[email protected]>
>
> Signed-off-by: Jiri Bohac <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> Reviewed-by: Jiri Bohac <[email protected]>
> cc: [email protected]
> ---
> arch/x86/Kconfig | 20 ++++++++---
> crypto/asymmetric_keys/verify_pefile.c | 4 ++-
> include/linux/kexec.h | 4 +--
> kernel/kexec_file.c | 47 ++++++++++++++++++++++----
> 4 files changed, 60 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index c1f9b3cf437c..84381dd60760 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2012,20 +2012,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..eec7e5bb2a08 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,47 @@ 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);
> + 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.22.0.410.gd8fdbe21b5-goog
>
On Sat, 22 Jun 2019, Pavel Machek wrote:
> > 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.
>
> I keep getting these...
>
> IIRC suse has patches to verify the images.
Yeah, Joey Lee is taking care of those. CCing.
--
Jiri Kosina
SUSE Labs
On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> From: David Howells <[email protected]>
>
> There are some bpf functions can be used to read kernel memory:
Nit: that
> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
Please explain how bpf_probe_write_user reads kernel memory ... ?!
> 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
... and while we're at it, also how they allow "kernel memory to be
altered without restriction". I've been pointing this false statement
out long ago.
> restriction. Disable them if the kernel has been locked down in
> confidentiality mode.
>
> Suggested-by: Alexei Starovoitov <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> Signed-off-by: Matthew Garrett <[email protected]>
> cc: [email protected]
> cc: Chun-Yi Lee <[email protected]>
> cc: Alexei Starovoitov <[email protected]>
> Cc: Daniel Borkmann <[email protected]>
Nacked-by: Daniel Borkmann <[email protected]>
[...]
>
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index d64c00afceb5..638f9b00a8df 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -137,6 +137,10 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
This whole thing is still buggy as has been pointed out before by
Jann. For helpers like above and few others below, error conditions
must clear the buffer ...
> ret = probe_kernel_read(dst, unsafe_ptr, size);
> if (unlikely(ret < 0))
> memset(dst, 0, size);
> @@ -156,6 +160,12 @@ static const struct bpf_func_proto bpf_probe_read_proto = {
> BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,
> u32, size)
> {
> + int ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * Ensure we're in user context which is safe for the helper to
> * run. This helper has no business in a kthread.
> @@ -205,7 +215,11 @@ BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
> int fmt_cnt = 0;
> u64 unsafe_addr;
> char buf[64];
> - int i;
> + int i, ret;
> +
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
>
> /*
> * bpf_check()->check_func_arg()->check_stack_boundary()
> @@ -534,6 +548,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
> {
> int ret;
>
> + ret = security_locked_down(LOCKDOWN_BPF_READ);
> + if (ret)
> + return ret;
> +
> /*
> * The strncpy_from_unsafe() call will likely not fill the entire
> * buffer, but that's okay in this circumstance as we're probing
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index 5a08c17f224d..2eea2cc13117 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_KCORE] = "/proc/kcore access",
> [LOCKDOWN_KPROBES] = "use of kprobes",
> + [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
>
>
On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <[email protected]> wrote:
>
> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > From: David Howells <[email protected]>
> >
> > There are some bpf functions can be used to read kernel memory:
>
> Nit: that
Fixed.
> > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
>
> Please explain how bpf_probe_write_user reads kernel memory ... ?!
Ha.
> > 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
>
> ... and while we're at it, also how they allow "kernel memory to be
> altered without restriction". I've been pointing this false statement
> out long ago.
Yup. How's the following description:
bpf: Restrict bpf when kernel lockdown is in confidentiality mode
There are some bpf functions that can be used to read kernel memory and
exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
bpf_trace_printk. These could be abused to (eg) allow private
keys in kernel
memory to be leaked. Disable them if the kernel has been locked
down in confidentiality
mode.
> This whole thing is still buggy as has been pointed out before by
> Jann. For helpers like above and few others below, error conditions
> must clear the buffer ...
Sorry, yes. My fault.
On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <[email protected]> wrote:
>
> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <[email protected]> wrote:
> >
> > On 06/22/2019 02:03 AM, Matthew Garrett wrote:
> > > From: David Howells <[email protected]>
> > >
> > > There are some bpf functions can be used to read kernel memory:
> >
> > Nit: that
>
> Fixed.
>
> > > bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
> >
> > Please explain how bpf_probe_write_user reads kernel memory ... ?!
>
> Ha.
>
> > > 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
> >
> > ... and while we're at it, also how they allow "kernel memory to be
> > altered without restriction". I've been pointing this false statement
> > out long ago.
>
> Yup. How's the following description:
>
> bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>
> There are some bpf functions that can be used to read kernel memory and
> exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
> bpf_trace_printk. These could be abused to (eg) allow private
> keys in kernel
> memory to be leaked. Disable them if the kernel has been locked
> down in confidentiality
> mode.
I'm confused. I understand why we're restricting bpf_probe_read().
Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
though?
--Andy
On Mon, Jun 24, 2019 at 1:09 PM Andy Lutomirski <[email protected]> wrote:
> I'm confused. I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?
Hmm. I think the thinking here was around exfiltration mechanisms, but
if the read is blocked then that seems less likely. This seems to
trace back to http://kernsec.org/pipermail/linux-security-module-archive/2017-October/003545.html
- Joey, do you know the reasoning here?
On 06/24/2019 10:08 PM, Andy Lutomirski wrote:
> On Mon, Jun 24, 2019 at 12:54 PM Matthew Garrett <[email protected]> wrote:
>> On Mon, Jun 24, 2019 at 8:37 AM Daniel Borkmann <[email protected]> wrote:
>>> On 06/22/2019 02:03 AM, Matthew Garrett wrote:
>>>> From: David Howells <[email protected]>
>>>>
>>>> There are some bpf functions can be used to read kernel memory:
>>>
>>> Nit: that
>>
>> Fixed.
>>
>>>> bpf_probe_read, bpf_probe_write_user and bpf_trace_printk. These allow
>>>
>>> Please explain how bpf_probe_write_user reads kernel memory ... ?!
>>
>> Ha.
>>
>>>> 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
>>>
>>> ... and while we're at it, also how they allow "kernel memory to be
>>> altered without restriction". I've been pointing this false statement
>>> out long ago.
>>
>> Yup. How's the following description:
>>
>> bpf: Restrict bpf when kernel lockdown is in confidentiality mode
>>
>> There are some bpf functions that can be used to read kernel memory and
>> exfiltrate it to userland: bpf_probe_read, bpf_probe_write_user and
>> bpf_trace_printk. These could be abused to (eg) allow private
>> keys in kernel
>> memory to be leaked. Disable them if the kernel has been locked
>> down in confidentiality
>> mode.
>
> I'm confused. I understand why we're restricting bpf_probe_read().
> Why are we restricting bpf_probe_write_user() and bpf_trace_printk(),
> though?
Agree, for example, bpf_probe_write_user() can never write into
kernel memory (only user one). Just thinking out loud, wouldn't it
be cleaner and more generic to perform this check at the actual function
which performs the kernel memory without faulting? All three of these
are in mm/maccess.c, and the very few occasions that override the
probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
so this would catch all of them wrt lockdown restrictions. Otherwise
you'd need to keep tracking every bit of new code being merged that
calls into one of these, no? That way you only need to do it once like
below and are guaranteed that the check catches these in future as well.
Thanks,
Daniel
diff --git a/mm/maccess.c b/mm/maccess.c
index 482d4d6..2c8220f 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -29,6 +29,9 @@ long __probe_kernel_read(void *dst, const void *src, size_t size)
long ret;
mm_segment_t old_fs = get_fs();
+ if (security_locked_down(LOCKDOWN_KERNEL_READ))
+ return -EFAULT;
+
set_fs(KERNEL_DS);
pagefault_disable();
ret = __copy_from_user_inatomic(dst,
@@ -57,6 +60,9 @@ long __probe_kernel_write(void *dst, const void *src, size_t size)
long ret;
mm_segment_t old_fs = get_fs();
+ if (security_locked_down(LOCKDOWN_KERNEL_WRITE))
+ return -EFAULT;
+
set_fs(KERNEL_DS);
pagefault_disable();
ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
@@ -90,6 +96,9 @@ long strncpy_from_unsafe(char *dst, const void *unsafe_addr, long count)
const void *src = unsafe_addr;
long ret;
+ if (security_locked_down(LOCKDOWN_KERNEL_READ))
+ return -EFAULT;
+
if (unlikely(count <= 0))
return 0;
On Mon, Jun 24, 2019 at 2:22 PM Daniel Borkmann <[email protected]> wrote:
> Agree, for example, bpf_probe_write_user() can never write into
> kernel memory (only user one). Just thinking out loud, wouldn't it
> be cleaner and more generic to perform this check at the actual function
> which performs the kernel memory without faulting? All three of these
> are in mm/maccess.c, and the very few occasions that override the
> probe_kernel_read symbol are calling eventually into __probe_kernel_read(),
> so this would catch all of them wrt lockdown restrictions. Otherwise
> you'd need to keep tracking every bit of new code being merged that
> calls into one of these, no? That way you only need to do it once like
> below and are guaranteed that the check catches these in future as well.
Not all paths into probe_kernel_read/write are from entry points that
need to be locked down (eg, as far as I can tell ftrace can't leak
anything interesting here).
On Fri, 21 Jun 2019, Matthew Garrett wrote:
> Minor updates over V33 - security_is_locked_down renamed to
> security_locked_down, return value of security_locked_down is returned
> in most cases, one unnecessary patch was dropped, couple of minor nits
> fixed.
Thanks for the respin.
We are still not resolved on granularity. Stephen has said he's not sure
if a useful policy can be constructed with just confidentiality and
integrity settings. I'd be interested to know JJ and Casey's thoughts on
lockdown policy flexibility wrt their respective LSMs.
These are also "all or nothing" choices which may prevent deployment due
to a user needing to allow (presumably controlled or mitigated) exceptions
to the policy.
--
James Morris
<[email protected]>
On 6/24/2019 4:01 PM, James Morris wrote:
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
>> Minor updates over V33 - security_is_locked_down renamed to
>> security_locked_down, return value of security_locked_down is returned
>> in most cases, one unnecessary patch was dropped, couple of minor nits
>> fixed.
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
Smack is a mandatory access control mechanism on named
objects controlled by the system. Issues of administrative
control, like whether hibernation is allowed, are outside
the scope of what Smack controls. There may be some subject/object
implications, but I have not identified any yet.
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
On Mon, Jun 24, 2019 at 4:01 PM James Morris <[email protected]> wrote:
>
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
> > Minor updates over V33 - security_is_locked_down renamed to
> > security_locked_down, return value of security_locked_down is returned
> > in most cases, one unnecessary patch was dropped, couple of minor nits
> > fixed.
>
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
This implementation provides arbitrary granularity at the LSM level,
though the lockdown LSM itself only provides two levels. Other LSMs
can choose an appropriate level of exposure.
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
Distributions have been deploying the "all or nothing" solution for
several years now, which implies that it's adequate for the common
case. I think it's reasonable to punt finer grained policies over to
other LSMs - people who want that are probably already using custom
LSM policy.
On 06/24/19 at 10:01am, Dave Young wrote:
> On 06/21/19 at 05:03pm, Matthew Garrett wrote:
> > 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 if there is a signature for which we
> > have a key, but signature doesn't match - even if in non-forcing mode.
> >
> > (3) kexec fails with EBADMSG or some other error if there is a signature
> > which cannot be parsed - even if in non-forcing mode.
> >
> > (4) kexec fails with ELIBBAD if the PE file cannot be parsed to extract
> > the signature - even if in non-forcing mode.
> >
> > ]
>
> Seems I do not see EBADMSG and ELIBBAD in this patch, also kexec fails
> with proper errno instead of EKEYREJECTED only.
>
> I may missed something? Other than the patch log issue:
>
> Reviewed-by: Dave Young <[email protected]>
Hold on :) Noticed another issue, please see comment inline..
>
> >
> > Signed-off-by: Jiri Bohac <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > Signed-off-by: Matthew Garrett <[email protected]>
> > Reviewed-by: Jiri Bohac <[email protected]>
> > cc: [email protected]
> > ---
> > arch/x86/Kconfig | 20 ++++++++---
> > crypto/asymmetric_keys/verify_pefile.c | 4 ++-
> > include/linux/kexec.h | 4 +--
> > kernel/kexec_file.c | 47 ++++++++++++++++++++++----
> > 4 files changed, 60 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> > index c1f9b3cf437c..84381dd60760 100644
> > --- a/arch/x86/Kconfig
> > +++ b/arch/x86/Kconfig
> > @@ -2012,20 +2012,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..eec7e5bb2a08 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,47 @@ 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;
Use -ENODATA for above case looks not correct, please just remove the #else and
move the #endif to the end of the switch chunk.
> > +#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);
> > + 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.22.0.410.gd8fdbe21b5-goog
> >
Thanks
Dave
On Mon, 24 Jun 2019, Matthew Garrett wrote:
> > We are still not resolved on granularity. Stephen has said he's not sure
> > if a useful policy can be constructed with just confidentiality and
> > integrity settings. I'd be interested to know JJ and Casey's thoughts on
> > lockdown policy flexibility wrt their respective LSMs.
>
> This implementation provides arbitrary granularity at the LSM level,
> though the lockdown LSM itself only provides two levels. Other LSMs
> can choose an appropriate level of exposure.
Ahh, OK, I only looked at the patchset description and had not looked at
V33 yet.
This is looking good.
--
James Morris
<[email protected]>
On 6/24/19 4:01 PM, James Morris wrote:
> On Fri, 21 Jun 2019, Matthew Garrett wrote:
>
>> Minor updates over V33 - security_is_locked_down renamed to
>> security_locked_down, return value of security_locked_down is returned
>> in most cases, one unnecessary patch was dropped, couple of minor nits
>> fixed.
>
> Thanks for the respin.
>
> We are still not resolved on granularity. Stephen has said he's not sure
> if a useful policy can be constructed with just confidentiality and
> integrity settings. I'd be interested to know JJ and Casey's thoughts on
> lockdown policy flexibility wrt their respective LSMs.
>
> These are also "all or nothing" choices which may prevent deployment due
> to a user needing to allow (presumably controlled or mitigated) exceptions
> to the policy.
>
>
I haven't gotten a chance to play with this the way I want to so there is
still a lot of questions regarding its interaction with apparmor and its
policy, but from what I have seen so far it is looking good.
I expect the all or nothing choices may limit its deployments (we really
need to play with this more to say) but we already face similar issues.
There are options we provide at a distro level that we can't turn on by
default, but we do recommend to more security conscious users.
If lockdown was in kernel we would certainly make it available for our
users, we have even had a few people ask about it.
On Sat, 22 Jun 2019 at 02:05, Matthew Garrett <[email protected]> wrote:
>
> efivar_ssdt_load allows the kernel to import arbitrary ACPI code from an
> EFI variable, which gives arbitrary code execution in ring 0. Prevent
> that when the kernel is locked down.
>
> Signed-off-by: Matthew Garrett <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Cc: [email protected]
Acked-by: Ard Biesheuvel <[email protected]>
> ---
> drivers/firmware/efi/efi.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index 55b77c576c42..9f92a013ab27 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -31,6 +31,7 @@
> #include <linux/acpi.h>
> #include <linux/ucs2_string.h>
> #include <linux/memblock.h>
> +#include <linux/security.h>
>
> #include <asm/early_ioremap.h>
>
> @@ -242,6 +243,11 @@ static void generic_ops_unregister(void)
> static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata;
> static int __init efivar_ssdt_setup(char *str)
> {
> + int ret = security_locked_down(LOCKDOWN_ACPI_TABLES);
> +
> + if (ret)
> + return ret;
> +
> if (strlen(str) < sizeof(efivar_ssdt))
> memcpy(efivar_ssdt, str, strlen(str));
> else
> --
> 2.22.0.410.gd8fdbe21b5-goog
>
Matthew Garrett <[email protected]> writes:
> 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]>
> ---
> include/linux/security.h | 1 +
> kernel/params.c | 27 ++++++++++++++++++++++-----
> security/lockdown/lockdown.c | 1 +
> 3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 61e3f4a62d16..88064d7f6827 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -92,6 +92,7 @@ enum lockdown_reason {
> LOCKDOWN_ACPI_TABLES,
> LOCKDOWN_PCMCIA_CIS,
> LOCKDOWN_TIOCSSERIAL,
> + LOCKDOWN_MODULE_PARAMETERS,
> LOCKDOWN_INTEGRITY_MAX,
> LOCKDOWN_CONFIDENTIALITY_MAX,
> };
> diff --git a/kernel/params.c b/kernel/params.c
> index ce89f757e6da..f94fe79e331d 100644
> --- a/kernel/params.c
> +++ b/kernel/params.c
> @@ -24,6 +24,7 @@
> #include <linux/err.h>
> #include <linux/slab.h>
> #include <linux/ctype.h>
> +#include <linux/security.h>
>
> #ifdef CONFIG_SYSFS
> /* Protects all built-in parameters, modules use their own param_lock */
> @@ -108,13 +109,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 &&
> + security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
> + return false;
> + return true;
> }
Should this test occur before tainting the kernel?
Regards,
Daniel
>
> static int parse_one(char *param,
> @@ -144,8 +151,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(¶ms[i]);
> - err = params[i].ops->set(val, ¶ms[i]);
> + if (param_check_unsafe(¶ms[i], doing))
> + err = params[i].ops->set(val, ¶ms[i]);
> + else
> + err = -EPERM;
> kernel_param_unlock(params[i].mod);
> return err;
> }
> @@ -553,6 +562,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 +580,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;
> diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
> index c89046dc2155..d03c4c296af7 100644
> --- a/security/lockdown/lockdown.c
> +++ b/security/lockdown/lockdown.c
> @@ -28,6 +28,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
> [LOCKDOWN_ACPI_TABLES] = "modified ACPI tables",
> [LOCKDOWN_PCMCIA_CIS] = "direct PCMCIA CIS storage",
> [LOCKDOWN_TIOCSSERIAL] = "reconfiguration of serial port IO",
> + [LOCKDOWN_MODULE_PARAMETERS] = "unsafe module parameters",
> [LOCKDOWN_INTEGRITY_MAX] = "integrity",
> [LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
> };
> --
> 2.22.0.410.gd8fdbe21b5-goog
On Fri, 21 Jun 2019, Matthew Garrett wrote:
> 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.
This is not a criticism of the patch but a related issue which I haven't
seen discussed (apologies if it has).
If signed code is loaded into ring 0, verified by the kernel, then
executed, you still lose your secure/trusted/verified boot state. If the
currently running kernel has been runtime-compromised, any signature
verification performed by the kernel cannot be trusted.
This problem is out of scope for the lockdown threat model (which
naturally cannot include a compromised kernel), but folk should be aware
that signature-verified kexec does not provide equivalent assurance to a
full reboot on a secure-boot system.
Potential mitigations here include runtime integrity verification of the
kernel via a separate security monitor (hypervisor, SMM, TEE etc.) or some
kind of platform support for kexec verification.
--
James Morris
<[email protected]>
On Wed, Jun 26, 2019 at 9:59 PM James Morris <[email protected]> wrote:
> This is not a criticism of the patch but a related issue which I haven't
> seen discussed (apologies if it has).
>
> If signed code is loaded into ring 0, verified by the kernel, then
> executed, you still lose your secure/trusted/verified boot state. If the
> currently running kernel has been runtime-compromised, any signature
> verification performed by the kernel cannot be trusted.
>
> This problem is out of scope for the lockdown threat model (which
> naturally cannot include a compromised kernel), but folk should be aware
> that signature-verified kexec does not provide equivalent assurance to a
> full reboot on a secure-boot system.
By that metric, on a secure boot system how do we determine that code
running in the firmware environment wasn't compromised before it
launched the initial signed kernel?
On Wed, Jun 26, 2019 at 6:49 PM Daniel Axtens <[email protected]> wrote:
>
> Matthew Garrett <[email protected]> writes:
> > + if (kp->flags & KERNEL_PARAM_FL_HWPARAM &&
> > + security_locked_down(LOCKDOWN_MODULE_PARAMETERS))
> > + return false;
> > + return true;
> > }
>
> Should this test occur before tainting the kernel?
Seems reasonable.
On Thu, 27 Jun 2019, Matthew Garrett wrote:
> By that metric, on a secure boot system how do we determine that code
> running in the firmware environment wasn't compromised before it
> launched the initial signed kernel?
Remote attestation tied to a hardware root of trust, before allowing
access to any further resources.
--
James Morris
<[email protected]>
On Thu, Jun 27, 2019 at 11:14 AM James Morris <[email protected]> wrote:
>
> On Thu, 27 Jun 2019, Matthew Garrett wrote:
>
> > By that metric, on a secure boot system how do we determine that code
> > running in the firmware environment wasn't compromised before it
> > launched the initial signed kernel?
>
> Remote attestation tied to a hardware root of trust, before allowing
> access to any further resources.
If you use IMA you can get the same guarantees over kexec.
Hi,
On Mon, Jun 24, 2019 at 03:21:23PM +0200, Jiri Kosina wrote:
> On Sat, 22 Jun 2019, Pavel Machek wrote:
>
> > > 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.
> >
> > I keep getting these...
> >
> > IIRC suse has patches to verify the images.
>
> Yeah, Joey Lee is taking care of those. CCing.
>
The last time that I sent for hibernation encryption and authentication is
here:
https://lkml.org/lkml/2019/1/3/281
It needs some big changes after review:
- Simplify the design: remove keyring dependency and trampoline.
- Encrypted whole snapshot image instead of only data pages.
- Using TPM:
- Direct use TPM API in hibernation instead of keyring
- Localities (suggested by James Bottomley)
I am still finding enough time to implement those changes, especial TPM
parts.
Thanks
Joey Lee
Hi experts,
On Mon, Jun 24, 2019 at 03:21:23PM +0200, Jiri Kosina wrote:
> On Sat, 22 Jun 2019, Pavel Machek wrote:
>
> > > 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.
> >
> > I keep getting these...
> >
> > IIRC suse has patches to verify the images.
>
> Yeah, Joey Lee is taking care of those. CCing.
>
The last time that I sent for hibernation encryption and authentication is
here:
https://lkml.org/lkml/2019/1/3/281
It needs some big changes after review:
- Simplify the design: remove keyring dependency and trampoline.
- Encrypted whole snapshot image instead of only data pages.
- Using TPM:
- Direct use TPM API in hibernation instead of keyring
- Localities (suggested by James Bottomley)
I am still finding enough time to implement those changes, especial TPM
parts.
Thanks
Joey Lee