Re: [PATCH v3 1/3] cryptodisk: make the password getter and additional argument to recover_key

[James Bottomley]

On Thu, 2020-12-31 at 21:42 +0300, Dmitry wrote:
[...]
> > --- a/grub-core/disk/luks2.c
> > +++ b/grub-core/disk/luks2.c
> > @@ -542,7 +542,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > 
> >  static grub_err_t
> >  luks2_recover_key (grub_disk_t source,
> > -                  grub_cryptodisk_t crypt)
> > +                  grub_cryptodisk_t crypt,
> > +                  grub_passwd_cb *password_get)
> 
> Do you have any thoughts for the future if we want to add luks header
> and master key passing to this function?

I really don't think you want to add luks header, because that takes
what is a generic interface and makes it luks specific.  You could add
some sort of opaque context instead, which the caller doesn't
understand, but the callee does, but I don't currently know how you
plan to use the header, so I have no idea if this would work or not.

> I'm using my own branch where I added this in a trivial way:
> static grub_err_t
> luks2_recover_key (grub_disk_t source,
>            grub_cryptodisk_t crypt,
>            grub_file_t hdr_file, grub_file_t key_file, grub_file_t
> mkey_file)
> 
> https://gitlab.com/reagentoo/grub/-/blob/cryptopatch_tiny_v2/grub-core/disk/luks2.c#L571-573
> 
> But I'm at a loss to think of how this can be done in combination
> with a 'grub_passwd_cb*'.

Well, we're both adding arguments to the function, so you just would
combine the additions, I think.

James