[RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

[RFC 0/4] Support kdump with LUKS encryption by reusing LUKS master key

[Coiby Xu]

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/e5abd089-3398-fdb4-7991-0019be434b79@gmail.com/
[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

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

[Coiby Xu]

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 <coxu@redhat.com>
---
 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

[RFC 2/4] kdump, x86: pass the LUKS master key to kdump kernel using a kernel command line parameter luksmasterkey

[Coiby Xu]

kdump will build up the kernel command parameter luksmasterkey as
similar to elfcorehdr to pass the memory address of the stored info of LUKS
master key to kdump kernel.

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 arch/x86/include/asm/crash.h      |  1 +
 arch/x86/kernel/crash.c           | 42 ++++++++++++++++++++++++++++++-
 arch/x86/kernel/kexec-bzimage64.c |  7 ++++++
 include/linux/kexec.h             |  4 +++
 4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/crash.h b/arch/x86/include/asm/crash.h
index 8b6bd63530dc..757374389296 100644
--- a/arch/x86/include/asm/crash.h
+++ b/arch/x86/include/asm/crash.h
@@ -4,6 +4,7 @@
 
 struct kimage;
 
+int crash_load_luks_key(struct kimage *image);
 int crash_load_segments(struct kimage *image);
 int crash_setup_memmap_entries(struct kimage *image,
 		struct boot_params *params);
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index e8326a8d1c5d..6d117da62da4 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -304,6 +304,7 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
 				 unsigned long long mend)
 {
 	unsigned long start, end;
+	int r;
 
 	cmem->ranges[0].start = mstart;
 	cmem->ranges[0].end = mend;
@@ -312,7 +313,19 @@ static int memmap_exclude_ranges(struct kimage *image, struct crash_mem *cmem,
 	/* Exclude elf header region */
 	start = image->elf_load_addr;
 	end = start + image->elf_headers_sz - 1;
-	return crash_exclude_mem_range(cmem, start, end);
+	r = crash_exclude_mem_range(cmem, start, end);
+
+	if (r)
+		return r;
+
+	/* Exclude LUKS master key region */
+	if (image->luks_master_key_addr) {
+		start = image->luks_master_key_addr;
+		end = start + image->luks_master_key_sz - 1;
+		return crash_exclude_mem_range(cmem, start, end);
+	}
+
+	return r;
 }
 
 /* Prepare memory map for crash dump kernel */
@@ -383,6 +396,33 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
 	return ret;
 }
 
+int crash_load_luks_key(struct kimage *image)
+{
+	int ret;
+	struct kexec_buf kbuf = { .image = image, .buf_min = 0,
+				  .buf_max = ULONG_MAX, .top_down = false };
+
+	image->luks_master_key_addr = 0;
+	ret = kexec_pass_luks_master_key(&kbuf.buffer, &kbuf.bufsz);
+	if (ret)
+		return ret;
+
+	kbuf.memsz = kbuf.bufsz;
+	kbuf.buf_align = ELF_CORE_HEADER_ALIGN;
+	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+	ret = kexec_add_buffer(&kbuf);
+	if (ret) {
+		vfree((void *)kbuf.buffer);
+		return ret;
+	}
+	image->luks_master_key_addr = kbuf.mem;
+	image->luks_master_key_sz = kbuf.bufsz;
+	pr_debug("Loaded LUKS master key at 0x%lx bufsz=0x%lx memsz=0x%lx\n",
+		 image->luks_master_key_addr, kbuf.bufsz, kbuf.bufsz);
+
+	return ret;
+}
+
 int crash_load_segments(struct kimage *image)
 {
 	int ret;
diff --git a/arch/x86/kernel/kexec-bzimage64.c b/arch/x86/kernel/kexec-bzimage64.c
index 170d0fd68b1f..64ea3b6a5768 100644
--- a/arch/x86/kernel/kexec-bzimage64.c
+++ b/arch/x86/kernel/kexec-bzimage64.c
@@ -76,6 +76,10 @@ static int setup_cmdline(struct kimage *image, struct boot_params *params,
 	if (image->type == KEXEC_TYPE_CRASH) {
 		len = sprintf(cmdline_ptr,
 			"elfcorehdr=0x%lx ", image->elf_load_addr);
+
+		if (image->luks_master_key_addr != 0)
+			len += sprintf(cmdline_ptr + len,
+					"luksmasterkey=0x%lx ", image->luks_master_key_addr);
 	}
 	memcpy(cmdline_ptr + len, cmdline, cmdline_len);
 	cmdline_len += len;
@@ -372,6 +376,9 @@ static void *bzImage64_load(struct kimage *image, char *kernel,
 		ret = crash_load_segments(image);
 		if (ret)
 			return ERR_PTR(ret);
+		ret = crash_load_luks_key(image);
+		if (ret)
+			pr_debug("Either no LUKS master key or error to retrieve the LUKS master key\n");
 	}
 
 	/*
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 91507bc684e2..456a5bc28518 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -316,6 +316,10 @@ struct kimage {
 	void *elf_headers;
 	unsigned long elf_headers_sz;
 	unsigned long elf_load_addr;
+
+	/* LUKS master key buffer */
+	unsigned long luks_master_key_addr;
+	unsigned long luks_master_key_sz;
 };
 
 /* kexec interface functions */
-- 
2.34.1

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

[Coiby Xu]

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

Signed-off-by: Coiby Xu <coxu@redhat.com>
---
 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

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

[Coiby Xu]

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 <coxu@redhat.com>
---
 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

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

[Milan Broz]

On 18/03/2022 11:34, Coiby Xu wrote:
> 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

Hi,

I think you are creating another attack vector here, storing the encryption
key to yet another place... but I already mentioned that in the referenced mail.

Why is it not done through keyring and forcing kdump to retain key there
(under the same keyring key name as dm-crypt used)?
Kernel dm-crypt supports this already; LUKS2 uses keyring by default too.
That's all you need, or not? Why do you need to add another "kdump:" thing?
IOW why kdump cannot copy the key to keyring under the name dm-crypt
has in the mapping table and let dm-crypt activate the device almost without
code changes?

Anyway, please fix the naming before this patchset can be read or reviewed!

LUKS is user-space key management only (on-disk metadata); the kernel has
no idea how the key is derived or what LUKS is - dm-crypt only knows the key
(either through keyring or directly in the mapping table).

Polluting kernel namespace with "luks" names variables is wrong - dm-crypt
is used in many other mappings (plain, bitlocker, veracrypt, ...)
Just use the dm-crypt key, do not reference LUKS at all.

Milan



>   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/e5abd089-3398-fdb4-7991-0019be434b79@gmail.com/
> [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(-)
> 

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

[Coiby Xu]

Hi Milan,

On Fri, Mar 18, 2022 at 12:29:06PM +0100, Milan Broz wrote:
>On 18/03/2022 11:34, Coiby Xu wrote:
>>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
>
>Hi,
>
>I think you are creating another attack vector here, storing the encryption
>key to yet another place... but I already mentioned that in the referenced mail.

Thanks for the quick response! I thought the take-home message of the
referenced is we shouldn't leave the master key in the user space. Sorry 
I seem to still misunderstand what's mentioned in the referenced mail. But
take AES as an example, my reasoning is as follows:
   the expanded key has already been stored in "struct crypto_aes_ctx".
   If a hacker can somehow access the master key stored in the memory
   reserved exclusively for kdump, it means he/she can also access the
   expanded key stored in struct crypto_aes_ctx. So  not creating
   another attack vector?

>
>Why is it not done through keyring and forcing kdump to retain key there
>(under the same keyring key name as dm-crypt used)?
>Kernel dm-crypt supports this already; LUKS2 uses keyring by default too.
>That's all you need, or not? Why do you need to add another "kdump:" thing?
>IOW why kdump cannot copy the key to keyring under the name dm-crypt
>has in the mapping table and let dm-crypt activate the device almost without
>code changes?

Sorry, I haven't explained how kdump works. Once the 1st kernel crashes and
the system boots into the kdump kernel, the kdump kernel only have direct
access to the memory exclusively reserved for it i.e. the kdump kernel
loses the direct access to the keyring constructed in the 1st kernel. In
theory, the kdump kernel could do some "hacking" to find out the key
stored in the memory directly managed by the 1st kernel but I imagine
this would be difficult task (imagine I present the memory dump of my
computer to you and ask you to rebuild all the relevant kernel data
structures and find the key). Besides, it's not reliable to read the
memory directly managed by the first kernel for example the memory could
be corrupt. So we have to pass the master key from the 1st kernel to the kdump
kernel.

Btw, if we store the key in the kernel keyring, doesn't it mean storing the
"encryption key to another place" and thus creating "another attach vector"?

>
>Anyway, please fix the naming before this patchset can be read or reviewed!
>
>LUKS is user-space key management only (on-disk metadata); the kernel has
>no idea how the key is derived or what LUKS is - dm-crypt only knows the key
>(either through keyring or directly in the mapping table).
>
>Polluting kernel namespace with "luks" names variables is wrong - dm-crypt
>is used in many other mappings (plain, bitlocker, veracrypt, ...)
>Just use the dm-crypt key, do not reference LUKS at all.

Thanks for pointing out my mistake of misusing the word LUKS! I'll fix this
error once we settle down what's the correct approach to reuse master
key for the kdump kernel.

>
>Milan
>
>
>
>>  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/e5abd089-3398-fdb4-7991-0019be434b79@gmail.com/
>>[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(-)
>>
>

-- 
Best regards,
Coiby

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

[Milan Broz]

On 18/03/2022 13:21, Coiby Xu wrote:
...
>> Why is it not done through keyring and forcing kdump to retain key there
>> (under the same keyring key name as dm-crypt used)?
>> Kernel dm-crypt supports this already; LUKS2 uses keyring by default too.
>> That's all you need, or not? Why do you need to add another "kdump:" thing?
>> IOW why kdump cannot copy the key to keyring under the name dm-crypt
>> has in the mapping table and let dm-crypt activate the device almost without
>> code changes?
> 
> Sorry, I haven't explained how kdump works. Once the 1st kernel crashes and
> the system boots into the kdump kernel, the kdump kernel only have direct
> access to the memory exclusively reserved for it i.e. the kdump kernel
> loses the direct access to the keyring constructed in the 1st kernel. In
> theory, the kdump kernel could do some "hacking" to find out the key
> stored in the memory directly managed by the 1st kernel but I imagine
> this would be difficult task (imagine I present the memory dump of my
> computer to you and ask you to rebuild all the relevant kernel data
> structures and find the key). Besides, it's not reliable to read the
> memory directly managed by the first kernel for example the memory could
> be corrupt. So we have to pass the master key from the 1st kernel to the kdump
> kernel.

OK, then why you cannot store it to the (2nd) kdump kernel keyring?
(From the kdump area copy, then you do not need to patch anything else
in dm-crypt than that one line storing the key to the kdump area.)

A clear approach would be to store the key in the 2nd kernel kdump keyring
and allow userspace to read it.
Then cryptsetup can just validate the key (LUKS key digest does not use Argon)
and activates it without asking for a passphrase.
Perhaps this will need some new cryptsetup option (or API call), but I think
it can be done.

Or, you can actually simulate it with
   cryptsetup open ... --master-key-file <file>
where this keyfile contains directly the volume key, not a passphrase.
The key digest is verified in this case only; no costly PBKDF is needed.

If you have a way to retrieve the kdump stored key to kdump userspace, this
is perhaps a much simpler solution.

All this is against all countermeasures to not expose encryption key
directly - but if kdump is debugging environment, just saying...

Milan

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

[Coiby Xu]

On Fri, Mar 18, 2022 at 02:53:37PM +0100, Milan Broz wrote:
>On 18/03/2022 13:21, Coiby Xu wrote:
>...
>>>Why is it not done through keyring and forcing kdump to retain key there
>>>(under the same keyring key name as dm-crypt used)?
>>>Kernel dm-crypt supports this already; LUKS2 uses keyring by default too.
>>>That's all you need, or not? Why do you need to add another "kdump:" thing?
>>>IOW why kdump cannot copy the key to keyring under the name dm-crypt
>>>has in the mapping table and let dm-crypt activate the device almost without
>>>code changes?
>>
>>Sorry, I haven't explained how kdump works. Once the 1st kernel crashes and
>>the system boots into the kdump kernel, the kdump kernel only have direct
>>access to the memory exclusively reserved for it i.e. the kdump kernel
>>loses the direct access to the keyring constructed in the 1st kernel. In
>>theory, the kdump kernel could do some "hacking" to find out the key
>>stored in the memory directly managed by the 1st kernel but I imagine
>>this would be difficult task (imagine I present the memory dump of my
>>computer to you and ask you to rebuild all the relevant kernel data
>>structures and find the key). Besides, it's not reliable to read the
>>memory directly managed by the first kernel for example the memory could
>>be corrupt. So we have to pass the master key from the 1st kernel to the kdump
>>kernel.
>
>OK, then why you cannot store it to the (2nd) kdump kernel keyring?
>(From the kdump area copy, then you do not need to patch anything else
>in dm-crypt than that one line storing the key to the kdump area.)

cryptsetup stores the master key in the thread keyring
(KEY_SPEC_THREAD_KEYRING). I couldn't come up with a easy way to store
the key to the (2nd) kdump kernel keyring. For example, one problem is
how can I know in adance the thread ID of cryptsetup? I imagine
drivers/md/dm-crypt.c needs to support a new type of message in
crypt_message so cryptsetup can tell dm-crypt to store the master key
in the thread keyring as a preparation for opening the device. So one
line change of code isn't sufficient. What's more important is I don't see 
any benefit by doing so I regards storing the key to the kdump kernel
keyring first as an unnecessary step.

>
>A clear approach would be to store the key in the 2nd kernel kdump keyring
>and allow userspace to read it.
>Then cryptsetup can just validate the key (LUKS key digest does not use Argon)
>and activates it without asking for a passphrase.

Thanks for bringing my attention to validating the key! Currently I
simply skip key validation in cryptsetup when implementing the
"--kdump-master-key" option because it requires pass the master key to
the user space. I have a side question, if somehow the master key is
changed and the wrong key is used to get the expanded key, will it lead
to a devastating effect say corrupting the user data on the disk?

>Perhaps this will need some new cryptsetup option (or API call), but I think
>it can be done.
>
>Or, you can actually simulate it with
>  cryptsetup open ... --master-key-file <file>
>where this keyfile contains directly the volume key, not a passphrase.
>The key digest is verified in this case only; no costly PBKDF is needed.
>
>If you have a way to retrieve the kdump stored key to kdump userspace, this
>is perhaps a much simpler solution.
>
>All this is against all countermeasures to not expose encryption key
>directly - but if kdump is debugging environment, just saying...

I respect and appreciate all the efforts done by you and other community
members to increase security margin so the last thing I want to do is to
reverse the security measures. Besides, I believe some users choose to
dump vmcore to an encrypted disk exactly out of security concern. The
only thing this patch set does about master key is to store it in the
memory exclusively reserved for kdump. To be precise, storing the master
key in the memory exclusively reserved for kdump may be different from
what you mean by "exposing encryption key directly". This reserved memory
won't be used by the 1st kernel (which means if we reserve 1GB more for
the kdump kernel, the 1st kernel would have 1GB less kernel to use. And I
would expect strong objection from the user due to a loss of 1GB memory).
Only kexec knows how to store the master key in this exclusively reserved
memory area and but kexec itself doesn't know to retrieve the master key in
the 1st kernel. So the situation is similar to how expanded key is stored.
struct crypto_aes_ctx has the expanded key, but there is no API to retrieve
it. If you still think this line of reasoning doesn't convince you, how
about encrypting the master key before storing the key to the kdump memory?
kexec could calculate the SHA256 digest of loaded kdump kernel and initrd.
We can use this SHA256 digest to get an encryption key and then encrypt
the master key.

>
>Milan
>

-- 
Best regards,
Coiby

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

[Guilherme G. Piccoli]

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

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

[Coiby Xu]

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
>kexec@lists.infradead.org
>http://lists.infradead.org/mailman/listinfo/kexec
>

-- 
Best regards,
Coiby

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

[Guilherme G. Piccoli]

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