Re: [PATCH] cryptodisk: allow the user to retry failed passphrases

[Forest]

On Thu, 4 Apr 2024 02:50:54 -0500, Glenn Washburn wrote:

>Are you getting dumped into the rescue shell or the normal shell? 

In my virtual machine, the prompt says:

grub rescue>

I don't think the distinction matters here, though, since GRUB shells and
scripts are beyond most of my friends and family members. They just need the
ability to retry a passphrase, with no special configuration, even if their
favorite linux distro didn't add scripts to implement retries.


>> +            ret = cr->recover_key (source, dev, cargs);
>
>There's a spacing issue here.

Thanks for spotting it. Fixed.


>> +        /* Clear the last error to avoid recover_key() retry failure. */
>> +        grub_errno = GRUB_ERR_NONE;
>
>Can you elaborate on what the "recover_key() retry failure." is?

Yes, I had already been considering that. Done. It now reads:

"Since recover_key() calls a function that returns grub_errno, a leftover
error value from a previously rejected passphrase will trigger a phantom
failure. We therefore clear it before trying a new passphrase."


>> +/* Number of passphrase attempts the user is allowed. Must be > 0. */
>> +#define GRUB_CRYPTODISK_PASSPHRASE_TRIES 3
>
>I'd suggest instead of having this hard coded, to instead read this
>value from a GRUB environment variable (maybe named
>grub_cryptodisk_passphrase_tries). This allows easy configuration from
>/etc/grub.d scripts.

For lack of documentation on how to do this, I followed the simpler examples
of environment variable reading that I found in existing code (biosnum.c and
pnvram.c). Done, I think.


I'll submit patch v2 shortly.