2022-03-18 15:32:34

by Coiby Xu

[permalink] [raw]
Subject: [RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

With kdump enabled, when kernel crashes, the system could boot into the
kdump kernel and dump the memory image i.e. /proc/vmcore to a specified
target. Currently, when dumping vmcore to a LUKS encrypted device, there
are two problems,
- for some machines, the user may don't have a chance enter the password
to decrypt the device after kernel crashes and kdump initrd is loaded
- LUKS2 by default use the memory-hard Argon2 key derivation function
which is quite memory-consuming compared to the limited memory reserved
for kdump. Take Fedora example, by default, only 256M is reserved for
systems having memory between 4G-64G. With LUKS enabled, ~1300M needs
to be reserved for kdump.

Besides the users (at least for Fedora) usually expect kdump to work out
of the box i.e. no manual password input is needed. And it doesn't make
sense to derivate the master key again in kdump kernel which seems to be
redundant work.

Based on Milan's feedback [1] on Kairui's ideas to support kdump with
LUKS encryption, this patch set addresses the above issues by
1) first saving the LUKS master key to kexec when opening the encrypted
device
2) then saving the master key to the reserved memory for kdump when
loading kdump kernel image.

So the LUKS master key never leaves the kernel space and once the key has
been saved to the reserved memory for kdump, it would be wiped
immediately. If there is no security concern with this approach or any
other concern, I will drop the following assumptions made for this RFC
version in v1,
- only x86 is supported
- there is only one LUKS device for the system

to extend the support to other architectures including POWER, ARM and
s390x and address the case of multiple LUKS devices. Any feedback will be
appreciated, thanks!

For a proof of concept, I've patched cryptsetup [2] in a quick-and-dirty
way to support a new option "--kdump-kernel-master-key"
and hacked systemd [3]. It works for Fedora 34.

[1] https://yhbt.net/lore/all/[email protected]/
[2] https://gitlab.com/coxu/cryptsetup/-/commit/ee54bb15445da0bc3f9155a7227a9799da4dac20
[3] https://github.com/coiby/systemd/tree/reuse_kdump_master_key

Coiby Xu (4):
kexec, dm-crypt: receive LUKS master key from dm-crypt and pass it to
kdump
kdump, x86: pass the LUKS master key to kdump kernel using a kernel
command line parameter luksmasterkey
crash_dump: retrieve LUKS master key in kdump kernel
dm-crypt: reuse LUKS master key in kdump kernel

arch/x86/include/asm/crash.h | 1 +
arch/x86/kernel/crash.c | 42 ++++++++++++++++++-
arch/x86/kernel/kexec-bzimage64.c | 7 ++++
drivers/md/dm-crypt.c | 26 +++++++++---
include/linux/crash_dump.h | 4 ++
include/linux/kexec.h | 7 ++++
kernel/crash_dump.c | 69 +++++++++++++++++++++++++++++++
kernel/kexec_core.c | 66 +++++++++++++++++++++++++++++
8 files changed, 215 insertions(+), 7 deletions(-)

--
2.34.1


2022-03-18 16:48:41

by Coiby Xu

[permalink] [raw]
Subject: [RFC 3/4] crash_dump: retrieve LUKS master key in kdump kernel

kdump will retrieve the LUKS master key based on the luksmasterkey
command line parameter.

Signed-off-by: Coiby Xu <[email protected]>
---
include/linux/crash_dump.h | 4 +++
kernel/crash_dump.c | 69 ++++++++++++++++++++++++++++++++++++++
2 files changed, 73 insertions(+)

diff --git a/include/linux/crash_dump.h b/include/linux/crash_dump.h
index 620821549b23..24acb84b716e 100644
--- a/include/linux/crash_dump.h
+++ b/include/linux/crash_dump.h
@@ -15,6 +15,8 @@
extern unsigned long long elfcorehdr_addr;
extern unsigned long long elfcorehdr_size;

+extern unsigned long long luks_master_key_addr;
+
#ifdef CONFIG_CRASH_DUMP
extern int elfcorehdr_alloc(unsigned long long *addr, unsigned long long *size);
extern void elfcorehdr_free(unsigned long long addr);
@@ -32,6 +34,8 @@ extern ssize_t copy_oldmem_page_encrypted(unsigned long pfn, char *buf,

void vmcore_cleanup(void);

+int retrive_kdump_luks_master_key(u8 *buffer, unsigned int *sz);
+
/* Architecture code defines this if there are other possible ELF
* machine types, e.g. on bi-arch capable hardware. */
#ifndef vmcore_elf_check_arch_cross
diff --git a/kernel/crash_dump.c b/kernel/crash_dump.c
index 92da32275af5..ee32de300b9e 100644
--- a/kernel/crash_dump.c
+++ b/kernel/crash_dump.c
@@ -15,6 +15,8 @@
unsigned long long elfcorehdr_addr = ELFCORE_ADDR_MAX;
EXPORT_SYMBOL_GPL(elfcorehdr_addr);

+unsigned long long luks_master_key_addr;
+EXPORT_SYMBOL_GPL(luks_master_key_addr);
/*
* stores the size of elf header of crash image
*/
@@ -39,3 +41,70 @@ static int __init setup_elfcorehdr(char *arg)
return end > arg ? 0 : -EINVAL;
}
early_param("elfcorehdr", setup_elfcorehdr);
+
+static int __init setup_luksmasterkey(char *arg)
+{
+ char *end;
+
+ if (!arg)
+ return -EINVAL;
+ luks_master_key_addr = memparse(arg, &end);
+ if (end > arg)
+ return 0;
+
+ luks_master_key_addr = 0;
+ return -EINVAL;
+}
+
+early_param("luksmasterkey", setup_luksmasterkey);
+
+/*
+ * Architectures may override this function to read LUKS master key
+ */
+ssize_t __weak luks_key_read(char *buf, size_t count, u64 *ppos)
+{
+ return read_from_oldmem(buf, count, ppos, 0, false);
+}
+
+int retrive_kdump_luks_master_key(u8 *buffer, unsigned int *sz)
+{
+ unsigned int key_size;
+ size_t lukskeybuf_sz;
+ unsigned int *size_ptr;
+ char *lukskeybuf;
+ u64 addr;
+ int r;
+
+ if (luks_master_key_addr == 0) {
+ pr_debug("LUKS master key memory address inaccessible");
+ return -EINVAL;
+ }
+
+ addr = luks_master_key_addr;
+
+ /* Read LUKS master key size */
+ r = luks_key_read((char *)&key_size, sizeof(unsigned int), &addr);
+
+ if (r < 0)
+ return r;
+
+ pr_debug("Retrieve LUKS master key: size=%u\n", key_size);
+ /* Read in LUKS maste rkey */
+ lukskeybuf_sz = sizeof(unsigned int) + key_size * sizeof(u8);
+ lukskeybuf = (void *)__get_free_pages(GFP_KERNEL | __GFP_ZERO,
+ get_order(lukskeybuf_sz));
+ if (!lukskeybuf)
+ return -ENOMEM;
+
+ addr = luks_master_key_addr;
+ r = luks_key_read((char *)lukskeybuf, lukskeybuf_sz, &addr);
+
+ if (r < 0)
+ return r;
+ size_ptr = (unsigned int *)lukskeybuf;
+ memcpy(buffer, size_ptr + 1, key_size * sizeof(u8));
+ pr_debug("Retrieve LUKS master key (size=%u): %48ph...\n", key_size, buffer);
+ *sz = key_size;
+ return 0;
+}
+EXPORT_SYMBOL(retrive_kdump_luks_master_key);
--
2.34.1

2022-03-18 19:39:50

by Coiby Xu

[permalink] [raw]
Subject: [RFC 1/4] kexec, dm-crypt: receive LUKS master key from dm-crypt and pass it to kdump

After receiving the LUKS master key from driver/md/dm-crypt, kdump has 1
hour at maximum to ask kexec to pass the key before the key gets wiped by
kexec. And after kdump retrieves the key, the key will be wiped
immediately.

Signed-off-by: Coiby Xu <[email protected]>
---
drivers/md/dm-crypt.c | 5 +++-
include/linux/kexec.h | 3 ++
kernel/kexec_core.c | 66 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index d4ae31558826..41f9ca377312 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -41,6 +41,7 @@
#include <keys/trusted-type.h>

#include <linux/device-mapper.h>
+#include <linux/kexec.h>

#include "dm-audit.h"

@@ -2388,6 +2389,8 @@ static int crypt_setkey(struct crypt_config *cc)
unsigned subkey_size;
int err = 0, i, r;

+ /* save master key to kexec */
+ kexec_save_luks_master_key(cc->key, cc->key_size);
/* Ignore extra keys (which are used for IV etc) */
subkey_size = crypt_subkey_size(cc);

@@ -3580,6 +3583,7 @@ static int crypt_message(struct dm_target *ti, unsigned argc, char **argv,
DMWARN("not suspended during key manipulation.");
return -EINVAL;
}
+
if (argc == 3 && !strcasecmp(argv[1], "set")) {
/* The key size may not be changed. */
key_size = get_key_size(&argv[2]);
@@ -3587,7 +3591,6 @@ static int crypt_message(struct dm_target *ti, unsigned argc, char **argv,
memset(argv[2], '0', strlen(argv[2]));
return -EINVAL;
}
-
ret = crypt_set_key(cc, argv[2]);
if (ret)
return ret;
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 0c994ae37729..91507bc684e2 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -205,6 +205,9 @@ int arch_kexec_locate_mem_hole(struct kexec_buf *kbuf);
extern int kexec_add_buffer(struct kexec_buf *kbuf);
int kexec_locate_mem_hole(struct kexec_buf *kbuf);

+extern int kexec_pass_luks_master_key(void **addr, unsigned long *sz);
+extern int kexec_save_luks_master_key(u8 *key, unsigned int key_size);
+
/* Alignment required for elf header segment */
#define ELF_CORE_HEADER_ALIGN 4096

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 68480f731192..86df36b71443 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -1218,3 +1218,69 @@ void __weak arch_kexec_protect_crashkres(void)

void __weak arch_kexec_unprotect_crashkres(void)
{}
+
+
+static u8 *luks_master_key;
+static unsigned int luks_master_key_size;
+
+void wipe_luks_master_key(void)
+{
+ if (luks_master_key) {
+ memset(luks_master_key, 0, luks_master_key_size * sizeof(u8));
+ kfree(luks_master_key);
+ luks_master_key = NULL;
+ }
+}
+
+static void _wipe_luks_master_key(struct work_struct *dummy)
+{
+ wipe_luks_master_key();
+}
+
+static DECLARE_DELAYED_WORK(wipe_luks_master_key_work, _wipe_luks_master_key);
+
+static unsigned __read_mostly wipe_key_delay = 3600; /* 1 hour */
+
+int kexec_save_luks_master_key(u8 *key, unsigned int key_size)
+{
+ if (luks_master_key) {
+ memset(luks_master_key, 0, luks_master_key_size * sizeof(u8));
+ kfree(luks_master_key);
+ }
+
+ luks_master_key = kmalloc(key_size * sizeof(u8), GFP_KERNEL);
+
+ if (!luks_master_key)
+ return -ENOMEM;
+ memcpy(luks_master_key, key, key_size * sizeof(u8));
+ luks_master_key_size = key_size;
+ pr_debug("LUKS master key (size=%u): %64ph\n", key_size, luks_master_key);
+ schedule_delayed_work(&wipe_luks_master_key_work,
+ round_jiffies_relative(wipe_key_delay * HZ));
+ return 0;
+}
+EXPORT_SYMBOL(kexec_save_luks_master_key);
+
+int kexec_pass_luks_master_key(void **addr, unsigned long *sz)
+{
+ unsigned long luks_key_sz;
+ unsigned char *buf;
+ unsigned int *size_ptr;
+
+ if (!luks_master_key)
+ return -EINVAL;
+
+ luks_key_sz = sizeof(unsigned int) + luks_master_key_size * sizeof(u8);
+
+ buf = vzalloc(luks_key_sz);
+ if (!buf)
+ return -ENOMEM;
+
+ size_ptr = (unsigned int *)buf;
+ memcpy(size_ptr, &luks_master_key_size, sizeof(unsigned int));
+ memcpy(size_ptr + 1, luks_master_key, luks_master_key_size * sizeof(u8));
+ *addr = buf;
+ *sz = luks_key_sz;
+ wipe_luks_master_key();
+ return 0;
+}
--
2.34.1

2022-03-21 07:24:07

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

On 18/03/2022 07:34, Coiby Xu wrote:
> [...]
> Based on Milan's feedback [1] on Kairui's ideas to support kdump with
> LUKS encryption, this patch set addresses the above issues by
> 1) first saving the LUKS master key to kexec when opening the encrypted
> device
> 2) then saving the master key to the reserved memory for kdump when
> loading kdump kernel image.
>
> So the LUKS master key never leaves the kernel space and once the key has
> been saved to the reserved memory for kdump, it would be wiped
> immediately. If there is no security concern with this approach or any
> other concern, I will drop the following assumptions made for this RFC
> version in v1,
> - only x86 is supported
> - there is only one LUKS device for the system
>
> to extend the support to other architectures including POWER, ARM and
> s390x and address the case of multiple LUKS devices. Any feedback will be
> appreciated, thanks!
>

Hi Coiby, thanks for the very interesting work!
I confess I didn't review the code as I have not much experience in
dm-crypt/key management, but I have a generic question related with the
motivation of the patch set.

My understanding is that one (the main?) motivation of this series would
be to protect the saved memory (vmcore) from being read by some
"unauthorized" entity - in order to achieve this goal, it is hereby
proposed to allow kdump kernel to access a memory-saved key and with
that, mount an encrypted volume, saving the vmcore over there correct?

So, what if instead of playing with the volume key, users with this
concern address that by reserving some *unencrypted partition* for
saving the vmcore, but then *encrypt the vmcore* itself! So, instead of
requiring saving a full-volume key, mount everything, risk data
corruption if something goes bad...we just have makedumpfile encrypting
the vmcore with some preloaded key (which might be saved inside the
kdump minimal intird, for example), and saving the encrypted file into a
clear/unencrypted volume? This way we also prevent excessive memory
consumption during kdump due to the lvm/dm-userspace paraphernalia usage.

Does it make sense or am I being silly or missing something?
Cheers,


Guilherme

2022-03-21 21:37:02

by Guilherme G. Piccoli

[permalink] [raw]
Subject: Re: [RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

On 20/03/2022 22:41, Coiby Xu wrote:
> [...]
>
> I believe some users have security concern for where to save vmcore.
> This use case exactly fits your description and your proposed solution
> shall be good for this type of users. But I think many more users may
> just choose to encrypt the hard drive when installing the system and
> they would naturally expect kdump to work for the case of full disk
> encryption. So your proposed solution may not address the latter case
> where there is a much large user base.
>

Thanks Coiby, makes sense, your idea is more generic and seems to
address all the use cases!
Cheers,


Guilherme

2022-03-21 21:53:17

by Coiby Xu

[permalink] [raw]
Subject: Re: [RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

On Sat, Mar 19, 2022 at 05:13:21PM -0300, Guilherme G. Piccoli wrote:
>On 18/03/2022 07:34, Coiby Xu wrote:
>> [...]
>> Based on Milan's feedback [1] on Kairui's ideas to support kdump with
>> LUKS encryption, this patch set addresses the above issues by
>> 1) first saving the LUKS master key to kexec when opening the encrypted
>> device
>> 2) then saving the master key to the reserved memory for kdump when
>> loading kdump kernel image.
>>
>> So the LUKS master key never leaves the kernel space and once the key has
>> been saved to the reserved memory for kdump, it would be wiped
>> immediately. If there is no security concern with this approach or any
>> other concern, I will drop the following assumptions made for this RFC
>> version in v1,
>> - only x86 is supported
>> - there is only one LUKS device for the system
>>
>> to extend the support to other architectures including POWER, ARM and
>> s390x and address the case of multiple LUKS devices. Any feedback will be
>> appreciated, thanks!
>>
>
>Hi Coiby, thanks for the very interesting work!

Hi Guilherme,

I'm glad this work interests you and thanks for sharing your thoughts!

>I confess I didn't review the code as I have not much experience in
>dm-crypt/key management, but I have a generic question related with the
>motivation of the patch set.
>
>My understanding is that one (the main?) motivation of this series would
>be to protect the saved memory (vmcore) from being read by some
>"unauthorized" entity - in order to achieve this goal, it is hereby
>proposed to allow kdump kernel to access a memory-saved key and with
>that, mount an encrypted volume, saving the vmcore over there correct?

>
>So, what if instead of playing with the volume key, users with this
>concern address that by reserving some *unencrypted partition* for
>saving the vmcore, but then *encrypt the vmcore* itself! So, instead of
>requiring saving a full-volume key, mount everything, risk data
>corruption if something goes bad...we just have makedumpfile encrypting
>the vmcore with some preloaded key (which might be saved inside the
>kdump minimal intird, for example), and saving the encrypted file into a
>clear/unencrypted volume? This way we also prevent excessive memory
>consumption during kdump due to the lvm/dm-userspace paraphernalia usage.

I believe some users have security concern for where to save vmcore.
This use case exactly fits your description and your proposed solution
shall be good for this type of users. But I think many more users may
just choose to encrypt the hard drive when installing the system and
they would naturally expect kdump to work for the case of full disk
encryption. So your proposed solution may not address the latter case
where there is a much large user base.

>
>Does it make sense or am I being silly or missing something?
>Cheers,
>
>
>Guilherme
>
>_______________________________________________
>kexec mailing list
>[email protected]
>http://lists.infradead.org/mailman/listinfo/kexec
>

--
Best regards,
Coiby

2022-03-21 23:19:38

by Coiby Xu

[permalink] [raw]
Subject: [RFC 4/4] dm-crypt: reuse LUKS master key in kdump kernel

When libcryptsetup passes key string starting with ":kdump", dm-crypt
will interpret it as reusing the LUKS master key in kdump kernel.

Signed-off-by: Coiby Xu <[email protected]>
---
drivers/md/dm-crypt.c | 21 ++++++++++++++++-----
1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 41f9ca377312..f3986036ec40 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -42,6 +42,7 @@

#include <linux/device-mapper.h>
#include <linux/kexec.h>
+#include <linux/crash_dump.h>

#include "dm-audit.h"

@@ -2602,13 +2603,17 @@ static int crypt_set_key(struct crypt_config *cc, char *key)
{
int r = -EINVAL;
int key_string_len = strlen(key);
+ bool retrieve_kdump_key = false;
+
+ if (is_kdump_kernel() && !strncmp(key, ":kdump", 5))
+ retrieve_kdump_key = true;

/* Hyphen (which gives a key_size of zero) means there is no key. */
- if (!cc->key_size && strcmp(key, "-"))
+ if (!retrieve_kdump_key && !cc->key_size && strcmp(key, "-"))
goto out;

/* ':' means the key is in kernel keyring, short-circuit normal key processing */
- if (key[0] == ':') {
+ if (!retrieve_kdump_key && key[0] == ':') {
r = crypt_set_keyring_key(cc, key + 1);
goto out;
}
@@ -2620,9 +2625,15 @@ static int crypt_set_key(struct crypt_config *cc, char *key)
kfree_sensitive(cc->key_string);
cc->key_string = NULL;

- /* Decode key from its hex representation. */
- if (cc->key_size && hex2bin(cc->key, key, cc->key_size) < 0)
- goto out;
+ if (retrieve_kdump_key) {
+ r = retrive_kdump_luks_master_key(cc->key, &cc->key_size);
+ if (r < 0)
+ goto out;
+ } else {
+ /* Decode key from its hex representation. */
+ if (cc->key_size && hex2bin(cc->key, key, cc->key_size) < 0)
+ goto out;
+ }

r = crypt_setkey(cc);
if (!r)
--
2.34.1