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

[Forest <forestix@nom.one>]

Please ignore this rev in favor of v4, coming soon.

(I want to simplify the stale-errno guard around grub_strtoul().)


On Sat, 27 Apr 2024 17:15:42 -0700, 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>
>---
> 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