On Wed, Jun 07, 2023 at 08:14:44AM +0200, Milan Broz wrote:
On 6/6/23 13:02, Coiby Xu wrote:
On Mon, Jun 05, 2023 at 09:09:49AM +0200, Milan Broz wrote:
On 6/5/23 04:31, Coiby Xu wrote:
Hi Eric and Milan,
On Sat, Jun 03, 2023 at 11:22:52AM +0200, Milan Broz wrote:
On 6/2/23 23:34, Eric Biggers wrote:
On Thu, Jun 01, 2023 at 03:24:39PM +0800, Coiby Xu wrote:
[PATCH 0/5] Support kdump with LUKS encryption by reusing LUKS volume key
The kernel has no concept of LUKS at all. It provides dm-crypt, which LUKS
happens to use. But LUKS is a userspace concept.
This is a kernel patchset, so why does it make sense for it to be talking about
LUKS at all? Perhaps you mean dm-crypt?
Exactly.
Thanks for raising the above concern! The use cases like CoreOS and
Confidential VMs explicitly want kdump to work for LUKS. And correct me
if I'm wrong, I think the two problems addressed by this patch set only
apply to LUKS so the kdump part of the kernel only cares about the LUKS
case. If there are use cases where similar approach is needed, I'll be
happy to make the solution more generic.
I had the same comment almost a year ago... and it still applies:
https://lore.kernel.org/all/c857dcf8-024e-ab8a-fd26-295ce2e0ae41@xxxxxxxxx/
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 the reminding! That comment was on the first RFC version. But
starting with "RFC v2", there is no longer any interaction with dm-crypt
(to save a copy of the LUKS volume key for the kdump kernel) and now I
make cryptsetup talks to the kdump part of the kernel via the sysfs to
reuse the volume key. So only the kdump part of the kernel needs to know
LUKS which is what it cares. Thus I don't think there is any kernel
namespace pollution now.
Hi,
I am sorry if I did understand correctly, but I thought that kdump is part
of the kernel.
Yes, there is the kernel part of the kdump although there is also the
userspace part to make the feature complete:)
I am trying to say that kernel generally has no concept of LUKS;
this is a userspace abstraction for key management.
Even the cryptsetup dm-crypt configuration mapping table generated from LUKS
has nothing LUKS special in it (only in DM-UUID as a name prefix).
So I do not understand why you need to mention LUKS even in kdump part.
Perhaps it is still only a naming problem, nothing more.
All you need is to preserve key and configuration parameters (for dm-crypt).
If it is set by cryptsetup, dmsetup, or any other way is not important - on this
kernel layer, it has nothing to do with LUKS key management metadata.
No problem if you support only LUKS in userspace, but really, all this machinery
should work for any dm-crypt devices. Perhaps your patch even works for it already.
Thanks for the explanation! After reflecting on your words for some
time, I realize I had an implicit assumption. I assumed is if I use a
name like dm_crypt_key instead of luks_volume_key, I need to support all
mappings like plain, bitlocker, veracrypt as mentioned by you and this
could mean much more efforts. So I'm not motivated to do that as
currently users only request kdump to work for LUKS.
Thanks, I think it is perfectly fine to implement just subset here.
[...]
My comment was just about proper naming in kernel, it is of course up to you
what you want to support in userspace (and even in kernel, extensions can
be added later).
Thanks for the confirmation!
Only LUKS2 uses keyring for volume key in dm-crypt as default option anyway.
Thanks for the info!
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.
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/
Milan
--
Best regards,
Coiby
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
