Message-ID: <Y9ogR0/zPRBGI1kq@MiWiFi-R3L-srv>
Date:   Wed, 1 Feb 2023 16:18:15 +0800
From:   Baoquan He <bhe@...hat.com>
To:     Coiby Xu <coxu@...hat.com>
Cc:     kexec@...ts.infradead.org, Milan Broz <gmazyland@...il.com>,
        Thomas Staudt <tstaudt@...ibm.com>,
        Kairui Song <ryncsn@...il.com>, dm-devel@...hat.com,
        Jan Pazdziora <jpazdziora@...hat.com>,
        Pingfan Liu <kernelfans@...il.com>,
        Dave Young <dyoung@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC v2 0/5] Support kdump with LUKS encryption by reusing LUKS
 volume key

Hi Coiby,

On 11/04/22 at 07:29pm, Coiby Xu wrote:
> RFC v2
>  - libcryptsetup interacts with the kernel via sysfs instead of "hacking"
>    dm-crypt 
>    - to save a kdump copy of the LUKS volume key in 1st kernel
>    - to add a logon key using the copy for libcryptsetup in kdump kernel [Milan]
>    - to avoid the incorrect usage of LUKS master key in dm-crypt [Milan]
>  - save the kdump copy of LUKS volume key randomly [Jan]
>  - mark the kdump copy inaccessible [Pingfan]
>  - Miscellaneous
>    - explain when operations related to the LUKS volume key happen [Jan]
>    - s/master key/volume key/g
>    - use crash_ instead of kexec_ as function prefix
>    - fix commit subject prefixes e.g. "x86, kdump" to x86/crash
> 
> With kdump enabled, when the 1st kernel crashes, the system could boot
> into the kdump/crash 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,

I went through this patchset, have some concerns. Do you mind sending a
formal patchset with some adjustment?

1) maybe put the luks key handling related to crash_dump out into
a separate file. e.g kernel/crash_dump_luks.c. This will reduce the code
complexity to other kdump code from luks handling. At least from code
reading side.
2) please rearrange your patches order so that generic luks handling at
the beginning. Then the x86 specific code comes next. Please refer to
below patchset. Like this, other arch can easily add its specific code
accordingly.

[PATCH v18 0/7] crash: Kernel handling of CPU and memory hot un/plug

And I am wondering
1) in what proportion the luks system is used,
2) in which cases the dumped vmcore have to be saved into luks disk,
   no workaround?
3) with the 1 hour aging of luks key, to what extent it's still
   worthwhile to add the support of luks kdumping? E.g for me, I have
   been using Fedora, and often start kexec-tools service when I need to
   do test kdump w/o reboot. With that, the key has aged.

I am not familiar with LUKS and it's key mechanism, my above concerns
could be shallow, anyone please help clarify if it does impact. Please
also show your support if this adding is really helpful.

Thanks
Baoquan

> 
>  - for some machines, a system administrator may not have a chance to
>    enter the password to decrypt the device in kdump initramfs after the
>    1st kernel crashes 
> 
>  - 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. Note if the memory reserved for kdump can't
>    be used by 1st kernel i.e. an user sees ~1300M memory missing in the
>    1st kernel. 
>  
> Besides 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 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 reusing
> the LUKS volume key in kdump kernel and here is the life cycle of this
> kdump copy of LUKS volume key,
> 
>  1. After the 1st kernel loads the initramfs during boot, systemd
>     asks for a passphrase from the user and uses it to de-crypt the LUKS
>     volume key
> 
>  2. After the 1st kernel saving the volume key as a logon key,
>     libcrytpsetup notifies the kernel to read this logon key and store a
>     temporary copy by writing the key description to
>     /sys/kernel/crash_luks_volume_key
> 
>  3. After switching to the real root fs, kdump.serivce is started and it 
>     loads the kdump kernel using the kexec_file_load syscall
> 
>  4. The kexec_file_load syscall saves the temporary copy of the volume
>     key to kdump reserved memory and wipe the copy.
> 
>  5. When the 1st kernel crashes and kdump kernel is booted,
>     libcryptsetup asks the kdump kernel to add a logon key using
>     the volume key stored in kdump reserved memory by writing the key
>     description to /sys/kernel/crash_luks_volume_key
> 
>  6. The system gets rebooted to the 1st kernel after dumping vmcore to
>     the LUKS encrypted device is finished
> 
> Note the kdump copy of LUKS volume key never leaves the kernel space and
> is saved in the memory area exclusively reserved for kdump where even
> the 1st kernel has no direct access. 
> 
> Milan's major concern [2] on previous version is "storing the encryption
> key to yet another place are creating another attack vector". To further
> secure this copy, two additional protections are added,
>  - save the copy randomly in kdump reserved memory as suggested by Jan
>  - clear the _PAGE_PRESENT flag of the page that stores the copy as
>    suggested by Pingfan
> 
> If there is no further security concern with this approach or any other
> concern, I will drop the following assumptions,
>   - 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 [3] in a quick-and-dirty
> way to support a new option "--kdump-kernel-master-key"
> and hacked systemd [4]. It works for Fedora 35.
> 
> [1] https://yhbt.net/lore/all/e5abd089-3398-fdb4-7991-0019be434b79@gmail.com/
> [2] https://lwn.net/ml/linux-kernel/c857dcf8-024e-ab8a-fd26-295ce2e0ae41@gmail.com/
> [3] https://gitlab.com/coxu/cryptsetup/-/commit/750a46d933fac82e0c994b5c41de40a0b8cac647
> [4] https://github.com/coiby/systemd/tree/reuse_kdump_master_key
> 
> Coiby Xu (5):
>   kexec_file: allow to place kexec_buf randomly
>   crash_dump: save the LUKS volume key temporarily
>   x86/crash: pass the LUKS volume key to kdump kernel
>   x86/crash: make the page that stores the LUKS volume key inaccessible
>   crash_dump: retrieve LUKS volume key in kdump kernel
> 
>  arch/x86/include/asm/crash.h       |   1 +
>  arch/x86/kernel/crash.c            |  47 ++++++-
>  arch/x86/kernel/kexec-bzimage64.c  |   7 +
>  arch/x86/kernel/machine_kexec_64.c |  16 +++
>  include/linux/crash_core.h         |   2 +
>  include/linux/crash_dump.h         |   2 +
>  include/linux/kexec.h              |   6 +
>  kernel/crash_dump.c                | 200 +++++++++++++++++++++++++++++
>  kernel/kexec_file.c                |  15 +++
>  kernel/ksysfs.c                    |  19 +++
>  10 files changed, 314 insertions(+), 1 deletion(-)
> 
> -- 
> 2.37.3
> 

Powered by blists - more mailing lists