Re: Patches to cryptomount (plain support, keyfiles and LUKS detached headers)

[John Lane]

On 13/06/15 06:27, Andrei Borzenkov wrote:
> В Fri, 12 Jun 2015 20:15:32 +0100
> John Lane <address@hidden> пишет:
>
>>> Sorry, we cannot accept patches which aren't sent to this ml by author.
>> I've attached the patches here. They apply clean to c945ca75.
> Sending them as patch series in mail body would make it easier to
> review and comment on them.
Ok, sure I'll do that. please bear with me...
I'll re-jig the patches so that the plain-mode stuff is separated.
May be a delay as I'll need to find the time to do it.
>
>>> I'm not sure that all features are good. For starters plain mode is just
>>> difficult to setup and use. Please provide usecases not already covered
>>> by current features.
>> My target was to establish LUKS volumes with detached headers and key
>> files and this is not already covered by current features.
>>
>> My specific use-case is booting secured systems where the boot
>> environment (Grub, LUKS headers and keys) is contained on removable
>> media such as a USB key. The non-removable hard-drive has no boot code
>> on it; it just appears as an unformatted disk unless the removable key
>> is used.
>>
>> To support this, it was necessary to add support to Grub for detached
>> LUKS headers and keys.
>>
>> I am aware of a number of other people enquiring about this specific
>> functionality so I am not alone in thinking it's a valid use-case.
>>
>> Regarding plain mode,  I don't understand why plain mode is "difficult
>> to setup and use". I did the work on plain mode at the same time because
>> one of the disks that I needed to work with was a plain mode disk. I
>> asked about the existing but non-functioning "peter/devmapper" branch
>> and spent some time trying to get that to work. In the end, and as I
>> understand how LUKS uses dm-crypt, it seemed better to re-use the
>> existing code base in the cryptodisk routines because this is more
>> current, used and tested. By doing that I was able to get it to work
>> very quickly.
>>
> The problem is not coding it. It is impossible to identify plain mode
> crypto-containers at run time. So it depends on user knowing (or
> guessing) which of disks to use. It is pure manual stuff which cannot
> be integrated in GRUB grub-install or grub-mkconfig.
>
> You have 5 disks each encrypted with different key that are plain/use
> detached header. How can you setup GRUB to automatically find out the
> right key/header for each disk?
>
> I personally would appreciate keyfile support as separate patch, not
> buried in plain mode support. Especially if it would be accompanied by
> grub-mkconfig changes to automate generation of grub.cfg to use it. 
I'll separate the plain from LUKS stuff as said above... might take a
little while to find some time though...
>> I've been using my changes in daily use since my original postings last
>> December. I've just updated to the latest head and the patches still
>> merge cleanly.
>>
>> I'd appreciate it if these changes could be considered. If any more
>> information would be useful please let me know.
>>
>
>> @@ -488,6 +496,7 @@ grub_cryptodisk_open (const char *name, grub_disk_t disk)
>>  
>>    if (grub_memcmp (name, "cryptouuid/", sizeof ("cryptouuid/") - 1) == 0)
>>      {
>> +      grub_crypto_uuid_dehyphenate((char *)name + sizeof ("cryptouuid/"));
> This alters original name passed to grub_disk_open. As already
> mentioned it is better to use comparison function that ignores hyphens.
The reason I did it that way was because it was easy, coming from the
point of view that I was only getting to know the code-base.
I simply converted the uuid at the first opportunity into the format
that the existing code base expects. That way the rest of the code can
carry on as if a uuid without hyphens was presented. I thought that was
pretty harmless given that the dehyphenate function does nothing to a
uuid that has no hyphens in it. Anyway, I will resubmit the patch by
email as described above - we can discuss it further under that...

>

signature.asc
Description: OpenPGP digital signature