Re: [PATCH v4] cryptodisk: allow user to retry failed passphrase

[Maximilian Stendler <m.stendler@fu-berlin.de>]

Hi,

I've had the same issue but build a different solution via scripting[0]:
The cryptomount and retries are implemented in a grub.cfg template. With
some shell-scripts the actual grub.cfg can be generated and put into a
tar used as a memdisk, additionally containing all necessary modules and
compressed.
The custom efi to be used in the esp is generated with `grub-mkimage`
and an initial.cfg, which decompresses and mounts the tar with the
modules and sources the actual grub.cfg.

The behavior  differs a bit from this solution: It has unlimited retries
(need to CTRL+ALT+DEL to reboot) and thus never enters a rescue shell,
it has a (configurable) 3-second timeout between tries and prints a
red-colored (but not localized) message on a wrong password.


With that being said, I am definitely all for having such behavior in
the bootx64.efi generated by grub-mkinstall with disk encryption
enabled. That is probably more likely to be picked up by distros and
reaching more end users than my badly named repo with a hacked solution. :')

Cheers
Max

[0] https://github.com/stendler/grub-fde
The repo also contains scripts to test such generated efi files with
qemu and a Containerfile for a grub build environment in a podman/docker
image to test generating with different grub versions.

On 2024-04-28 02:48, Forest wrote:
> Give the user a chance to re-enter their cryptodisk passphrase after a typo,
> rather than immediately failing (and likely dumping them into a grub shell).
> 
> By default, we allow 3 tries before giving up. A value in the
> cryptodisk_passphrase_tries environment variable will override this default.
> 
> The user can give up early by entering an empty passphrase, just as they
> could before this patch.
> 
> Signed-off-by: Forest <forestix@nom.one>
> ---
> Note: This is identical to the v3 patch. The change I had in mind proved unviable.
> 
>  docs/grub.texi              |  9 +++++
>  grub-core/disk/cryptodisk.c | 74 +++++++++++++++++++++++++++++--------
>  2 files changed, 67 insertions(+), 16 deletions(-)
> 
> diff --git a/docs/grub.texi b/docs/grub.texi
> index a225f9a88..6ac603a32 100644
> --- a/docs/grub.texi
> +++ b/docs/grub.texi
> @@ -3277,6 +3277,7 @@ These variables have special meaning to GRUB.
>  * color_normal::
>  * config_directory::
>  * config_file::
> +* cryptodisk_passphrase_tries::
>  * debug::
>  * default::
>  * fallback::
> @@ -3441,6 +3442,14 @@ processed by commands @command{configfile} (@pxref{configfile}) or @command{norm
>  (@pxref{normal}).  It is restored to the previous value when command completes.
>  
>  
> +@node cryptodisk_passphrase_tries
> +@subsection cryptodisk_passphrase_tries
> +
> +When prompting the user for a cryptodisk passphrase, allow this many attempts
> +before giving up. The default is @samp{3}. (The user can give up early by
> +entering an empty passphrase.)
> +
> +
>  @node debug
>  @subsection debug
>  
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 2246af51b..4fa7dc58d 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -17,6 +17,7 @@
>   */
>  
>  #include <grub/cryptodisk.h>
> +#include <grub/env.h>
>  #include <grub/mm.h>
>  #include <grub/misc.h>
>  #include <grub/dl.h>
> @@ -1063,6 +1064,8 @@ grub_cryptodisk_scan_device_real (const char *name,
>    grub_cryptodisk_dev_t cr;
>    struct cryptodisk_read_hook_ctx read_hook_data = {0};
>    int askpass = 0;
> +  unsigned long tries = 3;
> +  const char *tries_env;
>    char *part = NULL;
>  
>    dev = grub_cryptodisk_get_by_source_disk (source);
> @@ -1114,32 +1117,71 @@ grub_cryptodisk_scan_device_real (const char *name,
>      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))
> +	tries_env = grub_env_get ("cryptodisk_passphrase_tries");
> +	if (tries_env != NULL && tries_env[0] != '\0')
>  	  {
> -	    grub_error (GRUB_ERR_BAD_ARGUMENT, "passphrase not supplied");
> -	    goto error;
> +	    const char *p = NULL;
> +	    tries = grub_strtoul (tries_env, &p, 0);
> +	    if (*p != '\0')
> +	      {
> +		/* Account for grub_strtoul() ignoring trailing text. */
> +		grub_err_t err = grub_errno;
> +		if (p > tries_env && tries != ~0UL)
> +		  err = GRUB_ERR_BAD_NUMBER;
> +
> +		grub_error (err,
> +			    N_("non-numeric or invalid value for cryptodisk_passphrase_tries: `%s'"),
> +			    tries_env);
> +		goto error;
> +	      }
>  	  }
> -	cargs->key_len = grub_strlen ((char *) cargs->key_data);
> -      }
>  
> -    ret = cr->recover_key (source, dev, cargs);
> -    if (ret != GRUB_ERR_NONE)
> -      goto error;
> +	for (; tries > 0; tries--)
> +	  {
> +	    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);
> +	    if (ret == GRUB_ERR_NONE)
> +	      break;
> +	    if (ret != GRUB_ERR_ACCESS_DENIED || tries == 1)
> +	      goto error;
> +	    grub_puts_ (N_("Invalid passphrase."));
> +
> +	    /*
> +	     * 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.
> +	     */
> +	    grub_errno = GRUB_ERR_NONE;
> +	  }
> +      }
>  
>      ret = grub_cryptodisk_insert (dev, name, source);
>      if (ret != GRUB_ERR_NONE)

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel