On Mon, 09 May 2022 22:27:30 +0200 Josselin Poiret <d...@jpoiret.xyz> wrote:
> Hello everyone, > > Glenn Washburn <developm...@efficientek.com> writes: > > > I don't really like this, but it gets the job done and is a work-around > > for a peculiarity of the LUKS2 backend. The cheat mount code for > > cryptodisk does only calls scan() and not recover_key(). For LUKS1 scan > > will return a grub_cryptodisk_t with log_sector_size set, but LUKS2 > > will not. This is because for LUKS1 the log_sector_size is constant > > (LUKS1 also sets most of the other properties of the cryptodisk device, > > like crypto algorithm, because they are in the binary header). However, > > for LUKS2 the sector size (along with other properties) is in the json > > header, which isn't getting parsed in scan(). > > > > For single segment LUKS2 containers, scan() could get the sector size > > from the json segment object. The LUKS2 spec says that normal LUKS2 > > devices are single segment[1], so this should work in the the cases the > > care about (currently). scan() would not be able to fill in the other > > properties, like crypto algorithm, because that depends on the keyslot > > used, which needs key recovery to be determined. To avoid parsing the > > json data twice, once in scan() and once in recover_key(), which should > > be avoided, the parsed json object could be put in the grub_cryptodisk_t > > in scan(), and used and freed in recover_key(). We'd probably also want > > to add a way for grub_cryptodisk_t objects to get cleaned up by the > > backend using them, so that the json object could be freed even if > > recover_key() is never called. > > > > I think the above is the real fix, a moderate amount more work, and not > > something I'd expect Pierre-Louis to take up. So if we're not going to > > do this to get this functionality to work, we'll need a hack to get it > > working. However, I'd prefer a different one. > > > > I've not tested this, but it seems to me that we can set the > > log_sector_size field to GRUB_DISK_SECTOR_BITS _if_ it equals zero in > > grub_cryptodisk_cheat_insert(). This limits the hack to only GRUB > > host/user-space code. > > Regarding these last lines, it's also possible to directly ask dm for > the actual sector size when cheatmounting, as well as the crypto > algorithm, bypassing the whole issue of parsing the json and finding the > right slot. This is roughly what's done in patch 2 of [1], maybe this > workaround would be more to your liking? Hah! Yes, thanks for reminding me. I forgot I'd suggested this and its a better approach than what I suggested above. And probably the one I'd support, but I need to more thoroughly take a look at it. > I've distributed this patch to several people that were having issues on > GNU Guix and they've been happily using LUKS2 with GRUB with it. Yes, this does work and too much of a hack as-is. Regardless, your contribution is appreciated. I'd like to get your patch with the GRUB fs tests merged. Do you want to make the changes I suggested and send a new patch here? If not, at some point I'll probably make them myself and submit it to the list. > [1] https://lists.gnu.org/archive/html/grub-devel/2021-12/msg00079.html > (20211211122945.6326-1-...@jpoiret.xyz) Glenn _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
