On Fri, 29 Mar 2024 23:48:00 -0700
Forest <fores...@nom.one> wrote:
> Hi, folks.
>
> I have long wished that GRUB would give me a chance to retry whenever I
> make a typo in a LUKS passphrase, rather than immediately dumping me into
> the rescue shell, so I finally took a crack at it myself.
Are you getting dumped into the rescue shell or the normal shell? If in
normal, you can use grub script to do effectively the same thing by
looping the desired number of times and checking the return code of the
cryptmount command.
> Would this be a welcome addition?
I think this is a reasonable addition and something I'd considered
adding too.
> The patch is a bit easier to read in a viewer that ignores whitespace,
> since a good chunk of the changed lines are from reordering and indenting
> existing code.
>
> Signed-off-by: Forest <fores...@nom.one>
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -1063,6 +1063,7 @@
> grub_cryptodisk_dev_t cr;
> struct cryptodisk_read_hook_ctx read_hook_data = {0};
> int askpass = 0;
> + int tries;
> char *part = NULL;
>
> dev = grub_cryptodisk_get_by_source_disk (source);
> @@ -1114,33 +1115,48 @@
> if (!dev)
> continue;
>
> - if (!cargs->key_len)
> + if (cargs->key_len)
> + {
> + ret = cr->recover_key (source, dev, cargs);
> + if (ret != GRUB_ERR_NONE)
> + goto error;
> + }
> + else
> {
> /* Get the passphrase from the user, if no key data. */
> askpass = 1;
> - part = grub_partition_get_name (source->partition);
> - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
> - source->partition != NULL ? "," : "",
> - part != NULL ? part : N_("UNKNOWN"),
> - dev->uuid);
> - grub_free (part);
> -
> cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> if (cargs->key_data == NULL)
> goto error_no_close;
>
> - if (!grub_password_get ((char *) cargs->key_data,
> GRUB_CRYPTODISK_MAX_PASSPHRASE))
> + for (tries = GRUB_CRYPTODISK_PASSPHRASE_TRIES; tries > 0; tries--)
> {
> - grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
> - goto error;
> + part = grub_partition_get_name (source->partition);
> + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> source->name,
> + source->partition != NULL ? "," : "",
> + part != NULL ? part : N_("UNKNOWN"),
> + dev->uuid);
> + grub_free (part);
> +
> + if (!grub_password_get ((char *) cargs->key_data,
> GRUB_CRYPTODISK_MAX_PASSPHRASE))
> + {
> + grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
> + goto error;
> + }
> + cargs->key_len = grub_strlen ((char *) cargs->key_data);
> +
> + ret = cr->recover_key (source, dev, cargs);
There's a spacing issue here.
> + if (ret == GRUB_ERR_NONE)
> + break;
> + if (ret != GRUB_ERR_ACCESS_DENIED || tries == 1)
> + goto error;
> + grub_puts_ (N_("Invalid passphrase."));
> +
> + /* 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?
> }
> - cargs->key_len = grub_strlen ((char *) cargs->key_data);
> }
>
> - ret = cr->recover_key (source, dev, cargs);
> - if (ret != GRUB_ERR_NONE)
> - goto error;
> -
> ret = grub_cryptodisk_insert (dev, name, source);
> if (ret != GRUB_ERR_NONE)
> goto error;
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -64,6 +64,9 @@
>
> #define GRUB_CRYPTODISK_MAX_KEYFILE_SIZE 8192
>
> +/* 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.
Over all, looks pretty good though.
Glenn
> +
> struct grub_cryptodisk;
>
> typedef gcry_err_code_t
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
