Re: [PATCH 0/5] Support kdump with LUKS encryption by reusing LUKS volume key

From: Coiby Xu
Date: Fri Jun 09 2023 - 06:14:06 EST

On Thu, Jun 08, 2023 at 12:39:26PM +0200, Milan Broz wrote:
On 6/7/23 14:39, Coiby Xu wrote:
...
I do not think you need any cryptsetup patches, all you need is to write
decrypted volume key from LUKS metadata with
cryptsetup luksDump ---dump-volume-key -volume-key-file <out> <device>
(or any code equivalent with libcryptsetup), am I correct?

Correct me if I'm wrong, but I don't think there will be a safer way to
preserve key without patching cryptsetup. Actually the --dump-volume-key
approach has been proposed before and I agree with your conclusion [1]
on that approach i.e. "passing volume key this way is quite insecure".
Without patching cryptsetup, even if I save the volume key in the memory
reserved for the kdump kernel, I need to retrieve this key in the
userspace to unlock the LUKS device which may lead to quite a security
vulnerability.

Hm, where are the patches for cryptsetup, then? I am afraid we do not want
to add such specific things there.

Thanks for cleaning up the text to make the discussion easier! Sorry I
only mentioned it [3] in the cover letter and didn't provide one in
previous reply. [3] was done in a quick-and-dirty way (I plan to send a
formal merge request after finishing the kernel part) and there is no need
to read it. Let's me explain what [3] does here instead,
1) After unlocking the LUKS-encrypted device, if cryptsetup finds
/sys/kernel/crash_luks_volume_key exists, it will write the key
description of the volume key to it to notify the kernel to save a copy
of this logon key linked to its thread keyring for the kdump kernel
2) After the 1st kernel crashes, if crytpsetup finds it's in the kdump
kernel, instead of deriving the volume key from a passphrase, it
will write the key description to /sys/kernel/crash_luks_volume_key
to ask the kdump kernel to link the saved key to its thread keyring.

[3] https://gitlab.com/coxu/cryptsetup/-/commit/750a46d933fac82e0c994b5c41de40a0b8cac647


But we are just going to merge a patchset that changes how we use keyring
where you can tell cryptsetup to store/link key under some specific name
and to specific keyring
(see https://gitlab.com/cryptsetup/cryptsetup/-/merge_requests/492)
(Please talk to Red Hat cryptsetup maintainers for more info,
I just mentioned this mail to them today.)

Thanks for pointing me to the above MR which looks promising! Unlike
treating the kdump use case as a special case [3], it just provides a
generic way with the implemented options --link-vk-to-keyring and
--volume-key-keyring.


I respect the efforts from you and the cryptsetup community to make LUKS
as secure as possible. And kdump is used in product environment. Kdump
is to a server as a black box is to an aircraft. So by no means I want
to reverse the used security measures and patching cryptsetup can allow
to keep the security measures. One concern raised by you against "FRC
v1" was a copy of the LUKS volume key for the kdump kernel creates an
attack vector. I took this feedback seriously and have sought advice
from my colleagues to implement the countermeasures ([PATCH 1/5] and
[Patch 4/5]).

[1] https://yhbt.net/lore/all/e5abd089-3398-fdb4-7991-0019be434b79@xxxxxxxxx/

Yes, I appreciate that. And it is perfectly ok if your customers accept
the trade-off and security risk of handling the key this way.

Anyway, I feel we are going in circles here, and as it seems to be my fault,
I do not want to sound grumpy as I am perhaps missing some context.

Actually I should thank you for your patience! You have been always
offering your feedback on this work kindly and promptly starting with
the first proposed solution [1].


Could you please talk to internal RH cryptsetup maintainers first and discuss
your solution? They know what we can do here can help to find an acceptable
solution. (I added cc to Ondra.)

Sure, I'll talk to them first. Thanks for letting Ondra know!


Thanks,
Milan


--
Best regards,
Coiby